From 51f60887bda9bec827ae4aed38bedeaeae85d49d Mon Sep 17 00:00:00 2001 From: attiasas Date: Wed, 17 Jun 2026 14:51:28 +0300 Subject: [PATCH 1/9] link required dependencies --- go.mod | 10 ++++++++++ go.sum | 8 ++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 118c7b34..6e44008d 100644 --- a/go.mod +++ b/go.mod @@ -198,6 +198,16 @@ require ( sigs.k8s.io/yaml v1.6.0 // indirect ) +// replace github.com/jfrog/jfrog-client-go => /Users/assafa/code/jfrog/jfrog-client-go + +// replace github.com/jfrog/jfrog-cli-core/v2 => /Users/assafa/code/jfrog/jfrog-cli-core + +// attiasas:xray_component_resolution +replace github.com/jfrog/jfrog-client-go => github.com/attiasas/jfrog-client-go v0.0.0-20260617113537-5536e6ba8c42 + +// attiasas:expend_xray_manager +replace github.com/jfrog/jfrog-cli-core/v2 => github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617114425-0d9f6f50df64 + // replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260604085947-7c110b77b4b4 // replace github.com/gfleury/go-bitbucket-v1 => github.com/gfleury/go-bitbucket-v1 v0.0.0-20230825095122-9bc1711434ab diff --git a/go.sum b/go.sum index 359c541c..22ef0cd0 100644 --- a/go.sum +++ b/go.sum @@ -59,6 +59,10 @@ github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPd github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= +github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617114425-0d9f6f50df64 h1:b+3Io6f60cku95RoJOi0N0fmHWpcVq+nUgLtn9jNmmk= +github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617114425-0d9f6f50df64/go.mod h1:MZApLFA/8EyIA9BTGCDUw412bWZ4DXSfmqxwUWlparo= +github.com/attiasas/jfrog-client-go v0.0.0-20260617113537-5536e6ba8c42 h1:59+NXeZ5raJXnU/hQZ5lv36ry8D2wFGpfc8eXiAwhLE= +github.com/attiasas/jfrog-client-go v0.0.0-20260617113537-5536e6ba8c42/go.mod h1:FHpjN1nTDoj96xd6obe27EOgGErqzU0rQgC96L3Ch9E= github.com/aws/aws-sdk-go v1.55.8 h1:JRmEUbU52aJQZ2AjX4q4Wu7t4uZjOu71uyNmaWlUkJQ= github.com/aws/aws-sdk-go v1.55.8/go.mod h1:ZkViS9AqA6otK+JBBNH2++sx1sgxrPKcSzPPvQkUtXk= github.com/aws/aws-sdk-go-v2 v1.41.1 h1:ABlyEARCDLN034NhxlRUSZr4l71mh+T5KAeGh6cerhU= @@ -384,12 +388,8 @@ github.com/jfrog/froggit-go v1.21.1 h1:I/XUOO6GQ1d/rmBlM361F8T654C3ohIWrpw23xNL9 github.com/jfrog/froggit-go v1.21.1/go.mod h1:umBiakJB0CSPFfe0AHVaC3n9xsmUT7NGkDCny3bRchI= github.com/jfrog/gofrog v1.7.6 h1:QmfAiRzVyaI7JYGsB7cxfAJePAZTzFz0gRWZSE27c6s= github.com/jfrog/gofrog v1.7.6/go.mod h1:ntr1txqNOZtHplmaNd7rS4f8jpA5Apx8em70oYEe7+4= -github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260609101026-df3091b39d06 h1:A8hWKHyvqzGXfWmh+8lXv3waAkim4xiucBfGhl7ZOeQ= -github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260609101026-df3091b39d06/go.mod h1:9R90mhbczGXwW5EGlDs7F08ejQU/xdoDhYHMvzBiqgE= github.com/jfrog/jfrog-cli-evidence v0.9.0 h1:i9DhkQUxSZkhpp5oGR+N+SVAaqWDiUylbJcoDhM91uQ= github.com/jfrog/jfrog-cli-evidence v0.9.0/go.mod h1:R9faPfyQESBmKrdZCmHvlpmYSHmffswjNnFeT3RMq8I= -github.com/jfrog/jfrog-client-go v1.55.1-0.20260508101905-a17af78a38d7 h1:o8fk4yWLqNMldarXyh/4NbmdbYbuM+lKYobdJK7shqM= -github.com/jfrog/jfrog-client-go v1.55.1-0.20260508101905-a17af78a38d7/go.mod h1:sCE06+GngPoyrGO0c+vmhgMoVSP83UMNiZnIuNPzU8U= github.com/jmespath/go-jmespath v0.4.1-0.20220621161143-b0104c826a24 h1:liMMTbpW34dhU4az1GN0pTPADwNmvoRSeoZ6PItiqnY= github.com/jmespath/go-jmespath v0.4.1-0.20220621161143-b0104c826a24/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= github.com/kevinburke/ssh_config v1.6.0 h1:J1FBfmuVosPHf5GRdltRLhPJtJpTlMdKTBjRgTaQBFY= From 8328eddecf5da60ee90b79f1897dfa5e3c15f5bb Mon Sep 17 00:00:00 2001 From: attiasas Date: Wed, 17 Jun 2026 16:44:48 +0300 Subject: [PATCH 2/9] Basic service to allow Xray Heal before install --- artifactory/healcomponents/buildtool.go | 26 ++++ artifactory/healcomponents/buildtool_test.go | 30 ++++ artifactory/healcomponents/files.go | 63 ++++++++ artifactory/healcomponents/files_test.go | 32 ++++ artifactory/healcomponents/service.go | 96 ++++++++++++ artifactory/healcomponents/service_test.go | 145 +++++++++++++++++++ 6 files changed, 392 insertions(+) create mode 100644 artifactory/healcomponents/buildtool.go create mode 100644 artifactory/healcomponents/buildtool_test.go create mode 100644 artifactory/healcomponents/files.go create mode 100644 artifactory/healcomponents/files_test.go create mode 100644 artifactory/healcomponents/service.go create mode 100644 artifactory/healcomponents/service_test.go diff --git a/artifactory/healcomponents/buildtool.go b/artifactory/healcomponents/buildtool.go new file mode 100644 index 00000000..470be2fa --- /dev/null +++ b/artifactory/healcomponents/buildtool.go @@ -0,0 +1,26 @@ +package healcomponents + +import ( + "context" + "slices" +) + +// CommandRunner runs a build-tool subprocess (injected for tests). +type CommandRunner func(ctx context.Context, projectRoot string, args ...string) error + +// BuildTool describes a package manager for the generic resolution flow. +type BuildTool interface { + ToolName() string + RelevantCommands() []string + // ProjectRoot resolves monorepo/reactor/solution root from workingDir. + ProjectRoot(workingDir string) (string, error) + // EnsureLockfiles materializes expected lock artifacts when absent and the command allows it. + // Returns paths (relative to project root) that were bootstrapped and did not exist before. + EnsureLockfiles(ctx context.Context, projectRoot, command string, runner CommandRunner, bootstrapArgs ...string) (bootstrapped []string, err error) + // DiscoverLockfiles returns all lockfiles (paths relative to project root). + DiscoverLockfiles(workingDir string) ([]Lockfile, error) +} + +func IsRelevantCommand(tool BuildTool, command string) bool { + return slices.Contains(tool.RelevantCommands(), command) +} diff --git a/artifactory/healcomponents/buildtool_test.go b/artifactory/healcomponents/buildtool_test.go new file mode 100644 index 00000000..9dcaf3e9 --- /dev/null +++ b/artifactory/healcomponents/buildtool_test.go @@ -0,0 +1,30 @@ +package healcomponents + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +type fakeTool struct { + name string + commands []string +} + +func (f fakeTool) ToolName() string { return f.name } +func (f fakeTool) RelevantCommands() []string { return f.commands } +func (f fakeTool) ProjectRoot(dir string) (string, error) { return dir, nil } +func (f fakeTool) EnsureLockfiles(_ context.Context, _, _ string, _ CommandRunner, _ ...string) ([]string, error) { + return nil, nil +} +func (f fakeTool) DiscoverLockfiles(_ string) ([]Lockfile, error) { + return []Lockfile{{Path: "lock.json", Content: []byte(`{}`)}}, nil +} + +func TestIsRelevantCommand(t *testing.T) { + tool := fakeTool{name: "fake", commands: []string{"install", "ci"}} + assert.True(t, IsRelevantCommand(tool, "install")) + assert.True(t, IsRelevantCommand(tool, "ci")) + assert.False(t, IsRelevantCommand(tool, "publish")) +} diff --git a/artifactory/healcomponents/files.go b/artifactory/healcomponents/files.go new file mode 100644 index 00000000..92bad529 --- /dev/null +++ b/artifactory/healcomponents/files.go @@ -0,0 +1,63 @@ +package healcomponents + +import ( + "os" + "path/filepath" + + "github.com/jfrog/jfrog-client-go/utils/errorutils" +) + +type lockfileBackup struct { + path string + content []byte // nil means the file did not exist before apply +} + +// ApplyLockfiles backs up existing lockfiles under projectRoot, writes updates, returns restore func. +// Paths listed in treatAsAbsent are restored by deletion even if they exist on disk (bootstrapped locks). +func ApplyLockfiles(projectRoot string, lockfiles []Lockfile, treatAsAbsent []string) (restore func() error, err error) { + if len(lockfiles) == 0 { + return func() error { return nil }, nil + } + + absent := make(map[string]bool, len(treatAsAbsent)) + for _, path := range treatAsAbsent { + absent[path] = true + } + + var backups []lockfileBackup + for _, lf := range lockfiles { + fullPath := filepath.Join(projectRoot, lf.Path) + backup := lockfileBackup{path: fullPath} + if !absent[lf.Path] { + data, readErr := os.ReadFile(fullPath) + if readErr == nil { + backup.content = data + } else if !os.IsNotExist(readErr) { + return nil, errorutils.CheckError(readErr) + } + } + backups = append(backups, backup) + + if err = os.MkdirAll(filepath.Dir(fullPath), 0755); err != nil { + return nil, errorutils.CheckError(err) + } + if err = os.WriteFile(fullPath, lf.Content, 0644); err != nil { + return nil, errorutils.CheckError(err) + } + } + + return func() error { + for _, backup := range backups { + if backup.content == nil { + if err = os.Remove(backup.path); err != nil && !os.IsNotExist(err) { + return errorutils.CheckError(err) + } + continue + } + if err = os.WriteFile(backup.path, backup.content, 0644); err != nil { + return errorutils.CheckError(err) + } + } + return nil + }, nil +} diff --git a/artifactory/healcomponents/files_test.go b/artifactory/healcomponents/files_test.go new file mode 100644 index 00000000..d6cd9d51 --- /dev/null +++ b/artifactory/healcomponents/files_test.go @@ -0,0 +1,32 @@ +package healcomponents + +import ( + "os" + "path/filepath" + "testing" + + testsutils "github.com/jfrog/jfrog-client-go/utils/tests" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestApplyLockfiles_WritesMultipleFiles(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte("a"), 0644)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "app"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "app/gradle.lockfile"), []byte("b"), 0644)) + + restore, err := ApplyLockfiles(dir, []Lockfile{ + {Path: "package-lock.json", Content: []byte("a-healed")}, + {Path: "app/gradle.lockfile", Content: []byte("b-healed")}, + }, nil) + require.NoError(t, err) + defer testsutils.RemoveAllAndAssert(t, dir) + + a, _ := os.ReadFile(filepath.Join(dir, "package-lock.json")) + b, _ := os.ReadFile(filepath.Join(dir, "app/gradle.lockfile")) + assert.Equal(t, "a-healed", string(a)) + assert.Equal(t, "b-healed", string(b)) + + require.NoError(t, restore()) +} diff --git a/artifactory/healcomponents/service.go b/artifactory/healcomponents/service.go new file mode 100644 index 00000000..81a8d0eb --- /dev/null +++ b/artifactory/healcomponents/service.go @@ -0,0 +1,96 @@ +package healcomponents + +import ( + "context" + "encoding/json" + "os" + + "github.com/jfrog/jfrog-client-go/utils/errorutils" + "github.com/jfrog/jfrog-client-go/utils/log" + "github.com/jfrog/jfrog-client-go/xray/services" +) + +// HealComponentsDisabledEnvVar disables Xray heal components when set to "true". +// Feature is enabled by default; users run normal jf commands with no extra setup. +const HealComponentsDisabledEnvVar = "JFROG_CLI_HEAL_COMPONENTS_DISABLED" + +var noopRestore = func() error { return nil } + +func IsComponentResolutionDisabled() bool { + return os.Getenv(HealComponentsDisabledEnvVar) == "true" +} + +// Lockfile is a CLI-side lock artifact. Path is relative to project root (read/write only — not sent to Xray). +type Lockfile struct { + Path string + Content []byte +} + +// ComponentResolutionClient resolves a single lockfile via Xray. +type ComponentResolutionClient interface { + HealComponents(req services.ComponentResolutionRequest) (*services.ComponentResolutionResponse, error) +} + +// RunIfEnabled ensures lockfiles exist, discovers them, calls Xray once per file, writes healed lockfiles when changes returned. +// Returns a restore function to revert lockfile writes if the subsequent build-tool command fails. +func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo string, tool BuildTool, command, workingDir string, runner CommandRunner, bootstrapArgs ...string) (restore func() error, err error) { + if IsComponentResolutionDisabled() || !IsRelevantCommand(tool, command) { + log.Debug("Xray heal components disabled or not relevant command: ", command) + return noopRestore, nil + } + log.Debug("Running Xray heal components at '" + repo + "' RT repository for tool:", tool.ToolName()) + projectRoot, err := tool.ProjectRoot(workingDir) + if err != nil { + return noopRestore, err + } + log.Debug("Ensuring lockfiles in project root: ", projectRoot) + bootstrapped, err := tool.EnsureLockfiles(ctx, projectRoot, command, runner, bootstrapArgs...) + if err != nil { + return noopRestore, err + } + lockfiles, err := tool.DiscoverLockfiles(workingDir) + if err != nil { + return noopRestore, err + } + log.Debug("Discovered lockfiles: ", getLockfilePaths(lockfiles)) + var toWrite []Lockfile + var totalChanges int + for _, lf := range lockfiles { + resp, err := client.HealComponents(services.ComponentResolutionRequest{ + BuildTool: tool.ToolName(), + Repo: repo, + Lockfile: json.RawMessage(lf.Content), + }) + if err != nil { + return noopRestore, errorutils.CheckError(err) + } + if len(resp.Changes) == 0 { + log.Debug("No changes for ", lf.Path) + continue + } + toWrite = append(toWrite, Lockfile{Path: lf.Path, Content: resp.Content}) + totalChanges += len(resp.Changes) + log.Debug("Healed ", lf.Path, " with ", len(resp.Changes), " package change(s)") + for _, ch := range resp.Changes { + log.Debug(" ", ch.Package, ": ", ch.BeforeIntegrity, " → ", ch.AfterIntegrity) + } + } + if len(toWrite) == 0 { + return noopRestore, nil + } + log.Debug("Applying", len(toWrite), "healed lockfile(s)...") + restore, err = ApplyLockfiles(projectRoot, toWrite, bootstrapped) + if err != nil { + return noopRestore, err + } + log.Info("Xray component resolution healed ", totalChanges, " package change(s) across ", len(toWrite), " lockfile(s)") + return restore, nil +} + +func getLockfilePaths(lockfiles []Lockfile) []string { + paths := make([]string, 0, len(lockfiles)) + for _, lf := range lockfiles { + paths = append(paths, lf.Path) + } + return paths +} diff --git a/artifactory/healcomponents/service_test.go b/artifactory/healcomponents/service_test.go new file mode 100644 index 00000000..50753cff --- /dev/null +++ b/artifactory/healcomponents/service_test.go @@ -0,0 +1,145 @@ +package healcomponents + +import ( + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/jfrog/jfrog-client-go/xray/services" +) + +type mockClient struct { + callCount int + lastReq services.ComponentResolutionRequest + resp services.ComponentResolutionResponse +} + +func (m *mockClient) HealComponents(req services.ComponentResolutionRequest) (*services.ComponentResolutionResponse, error) { + m.callCount++ + m.lastReq = req + return &m.resp, nil +} + +type mockTool struct { + root string + lockfiles []Lockfile + ensureErr error +} + +func (m mockTool) ToolName() string { return "npm" } +func (m mockTool) RelevantCommands() []string { return []string{"install", "ci"} } +func (m mockTool) ProjectRoot(_ string) (string, error) { + return m.root, nil +} +func (m mockTool) EnsureLockfiles(_ context.Context, _, _ string, _ CommandRunner, _ ...string) ([]string, error) { + if m.ensureErr != nil { + return nil, m.ensureErr + } + return nil, nil +} +func (m mockTool) DiscoverLockfiles(_ string) ([]Lockfile, error) { + return m.lockfiles, nil +} + +func TestIsComponentResolutionDisabled(t *testing.T) { + t.Run("enabled by default", func(t *testing.T) { + t.Setenv(ComponentResolutionDisabledEnvVar, "") + assert.False(t, IsComponentResolutionDisabled()) + }) + t.Run("disabled when env true", func(t *testing.T) { + t.Setenv(ComponentResolutionDisabledEnvVar, "true") + assert.True(t, IsComponentResolutionDisabled()) + }) + t.Run("not disabled for other values", func(t *testing.T) { + t.Setenv(ComponentResolutionDisabledEnvVar, "false") + assert.False(t, IsComponentResolutionDisabled()) + }) +} + +func TestRunIfEnabled_WritesHealedLockfiles(t *testing.T) { + dir := t.TempDir() + lockPath := filepath.Join(dir, "package-lock.json") + require.NoError(t, os.WriteFile(lockPath, []byte("orig"), 0644)) + + client := &mockClient{resp: services.ComponentResolutionResponse{ + Content: json.RawMessage(`"healed"`), + Changes: []services.Change{{Package: "lodash", BeforeIntegrity: "a", AfterIntegrity: "b"}}, + }} + tool := mockTool{root: dir, lockfiles: []Lockfile{{Path: "package-lock.json", Content: []byte("orig")}}} + + _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", dir, nil) + require.NoError(t, err) + assert.Equal(t, 1, client.callCount) + assert.Equal(t, "npm", client.lastReq.BuildTool) + assert.Equal(t, "npm-virtual", client.lastReq.Repo) + + data, err := os.ReadFile(lockPath) + require.NoError(t, err) + assert.Equal(t, `"healed"`, string(data)) +} + +func TestRunIfEnabled_SkipsWhenDisabled(t *testing.T) { + t.Setenv(ComponentResolutionDisabledEnvVar, "true") + client := &mockClient{} + tool := mockTool{} + _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", t.TempDir(), nil) + require.NoError(t, err) + assert.Equal(t, 0, client.callCount) +} + +func TestRunIfEnabled_SkipsWhenNoChanges(t *testing.T) { + dir := t.TempDir() + lockPath := filepath.Join(dir, "package-lock.json") + require.NoError(t, os.WriteFile(lockPath, []byte("orig"), 0644)) + + client := &mockClient{resp: services.ComponentResolutionResponse{ + Content: json.RawMessage(`"orig"`), + Changes: nil, + }} + tool := mockTool{root: dir, lockfiles: []Lockfile{{Path: "package-lock.json", Content: []byte("orig")}}} + + _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", dir, nil) + require.NoError(t, err) + + data, err := os.ReadFile(lockPath) + require.NoError(t, err) + assert.Equal(t, "orig", string(data)) +} + +func TestRunIfEnabled_LoopsPerDiscoveredFile(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "a.lock"), []byte("a"), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "b.lock"), []byte("b"), 0644)) + + client := &mockClient{resp: services.ComponentResolutionResponse{Content: json.RawMessage(`"x"`), Changes: nil}} + tool := mockTool{root: dir, lockfiles: []Lockfile{ + {Path: "a.lock", Content: []byte("a")}, + {Path: "b.lock", Content: []byte("b")}, + }} + _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", dir, nil) + require.NoError(t, err) + assert.Equal(t, 2, client.callCount) +} + +func TestRunIfEnabled_SkipsIrrelevantCommand(t *testing.T) { + client := &mockClient{} + tool := mockTool{} + _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "version", t.TempDir(), nil) + require.NoError(t, err) + assert.Equal(t, 0, client.callCount) +} + +func TestRunIfEnabled_PropagatesEnsureLockfilesError(t *testing.T) { + client := &mockClient{} + tool := mockTool{ensureErr: errors.New("package-lock.json is required for npm ci")} + _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "ci", t.TempDir(), nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "package-lock.json") + assert.Equal(t, 0, client.callCount) +} From 064cb23e72d3f1fc1f56535015b3ca762651de2d Mon Sep 17 00:00:00 2001 From: attiasas Date: Wed, 17 Jun 2026 18:59:47 +0300 Subject: [PATCH 3/9] support npm --- artifactory/commands/npm/common.go | 12 + artifactory/commands/npm/npmcommand.go | 33 ++- artifactory/commands/npm/publish.go | 13 +- artifactory/commands/npm/xrayheal.go | 241 +++++++++++++++++++++ artifactory/commands/npm/xrayheal_test.go | 132 +++++++++++ artifactory/healcomponents/service.go | 20 +- artifactory/healcomponents/service_test.go | 23 +- go.mod | 4 +- go.sum | 8 +- 9 files changed, 454 insertions(+), 32 deletions(-) create mode 100644 artifactory/commands/npm/xrayheal.go create mode 100644 artifactory/commands/npm/xrayheal_test.go diff --git a/artifactory/commands/npm/common.go b/artifactory/commands/npm/common.go index ef1acde7..4aa59e5f 100644 --- a/artifactory/commands/npm/common.go +++ b/artifactory/commands/npm/common.go @@ -14,6 +14,8 @@ type CommonArgs struct { npmArgs []string serverDetails *config.ServerDetails useNative bool + configFilePath string + executablePath string } func (ca *CommonArgs) SetServerDetails(serverDetails *config.ServerDetails) *CommonArgs { @@ -45,6 +47,16 @@ func (ca *CommonArgs) SetUseNative(useNpmRc bool) *CommonArgs { return ca } +func (ca *CommonArgs) SetConfigFilePath(configFilePath string) *CommonArgs { + ca.configFilePath = configFilePath + return ca +} + +func (ca *CommonArgs) SetExecutablePath(executablePath string) *CommonArgs { + ca.executablePath = executablePath + return ca +} + // CheckIsNativeAndFetchFilteredArgs checks if native mode should be enabled. // It first checks the JFROG_RUN_NATIVE environment variable (preferred), // then falls back to the deprecated --run-native flag for backward compatibility. diff --git a/artifactory/commands/npm/npmcommand.go b/artifactory/commands/npm/npmcommand.go index 47ae45e5..ed93b760 100644 --- a/artifactory/commands/npm/npmcommand.go +++ b/artifactory/commands/npm/npmcommand.go @@ -2,6 +2,7 @@ package npm import ( "bufio" + "context" "errors" "fmt" "net/http" @@ -56,9 +57,8 @@ var ( type NpmCommand struct { CommonArgs - cmdName string - jsonOutput bool - executablePath string + cmdName string + jsonOutput bool // Function to be called to restore the user's old npmrc and delete the one we created. restoreNpmrcFunc func() error workingDirectory string @@ -73,6 +73,8 @@ type NpmCommand struct { collectBuildInfo bool buildInfoModule *build.NpmModule installHandler *NpmInstallStrategy + // When true, the subsequent install uses npm ci to honor healed lockfile integrity. + healedLockfile bool // When true, skips the 404 error handling that checks if packages are blocked by curation disableCVSCheck bool } @@ -135,7 +137,7 @@ func (nc *NpmCommand) Init() error { if err != nil { return err } - + repoConfig, err := nc.getRepoConfig(vConfig) if err != nil { return err @@ -348,6 +350,16 @@ func (nc *NpmCommand) Run() (err error) { if err = nc.PreparePrerequisites(nc.repo); err != nil { return } + var restoreResolution func() error + restoreResolution, nc.healedLockfile, err = nc.runXrayComponentHealing(context.Background(), nc.cmdName, nc.workingDirectory, nc.npmArgs) + if err != nil { + return err + } + defer func() { + if err != nil && restoreResolution != nil { + err = errors.Join(err, restoreResolution()) + } + }() defer func() { err = errors.Join(err, nc.installHandler.RestoreNpmrc()) }() @@ -508,8 +520,19 @@ func (nc *NpmCommand) prepareBuildInfoModule() error { return nil } +func (nc *NpmCommand) effectiveNpmCommand() string { + if nc.healedLockfile && nc.cmdName == "install" { + return "ci" + } + return nc.cmdName +} + func (nc *NpmCommand) collectDependencies() error { - nc.buildInfoModule.SetNpmArgs(append([]string{nc.cmdName}, nc.npmArgs...)) + npmCommand := nc.effectiveNpmCommand() + if npmCommand != nc.cmdName { + log.Info("Using npm ci after component resolution to install from the healed lockfile") + } + nc.buildInfoModule.SetNpmArgs(append([]string{npmCommand}, nc.npmArgs...)) return errorutils.CheckError(nc.buildInfoModule.Build()) } diff --git a/artifactory/commands/npm/publish.go b/artifactory/commands/npm/publish.go index 2624a435..2b3f9895 100644 --- a/artifactory/commands/npm/publish.go +++ b/artifactory/commands/npm/publish.go @@ -3,6 +3,7 @@ package npm import ( "archive/tar" "compress/gzip" + "context" "encoding/json" "errors" "fmt" @@ -37,7 +38,6 @@ const ( type NpmPublishCommandArgs struct { CommonArgs - executablePath string workingDirectory string collectBuildInfo bool packedFilePaths []string @@ -79,6 +79,7 @@ func (npc *NpmPublishCommand) ServerDetails() (*config.ServerDetails, error) { func (npc *NpmPublishCommand) SetConfigFilePath(configFilePath string) *NpmPublishCommand { npc.configFilePath = configFilePath + npc.SetConfigFilePath(configFilePath) return npc } @@ -172,6 +173,16 @@ func (npc *NpmPublishCommand) Run() (err error) { if err != nil { return err } + var restoreResolution func() error + restoreResolution, _, err = npc.runXrayComponentHealing(context.Background(), "publish", npc.workingDirectory, npc.npmArgs) + if err != nil { + return err + } + defer func() { + if err != nil && restoreResolution != nil { + err = errors.Join(err, restoreResolution()) + } + }() var npmBuild *build.Build var buildName, buildNumber, projectKey string diff --git a/artifactory/commands/npm/xrayheal.go b/artifactory/commands/npm/xrayheal.go new file mode 100644 index 00000000..b04d2bd0 --- /dev/null +++ b/artifactory/commands/npm/xrayheal.go @@ -0,0 +1,241 @@ +package npm + +import ( + "context" + "encoding/json" + "strings" + "os" + "fmt" + "path/filepath" + + "github.com/jfrog/jfrog-client-go/utils/errorutils" + "github.com/jfrog/jfrog-client-go/utils/log" + + gofrogcmd "github.com/jfrog/gofrog/io" + + "github.com/jfrog/jfrog-cli-core/v2/common/project" + "github.com/jfrog/jfrog-cli-core/v2/utils/xray" + + "github.com/jfrog/jfrog-cli-artifactory/artifactory/healcomponents" +) + +const ( + toolName = "npm" + lockfileName = "package-lock.json" +) + +func (ca *CommonArgs) runXrayComponentHealing(ctx context.Context, command, workingDir string, npmArgs []string) (restore func() error, healed bool, err error) { + noop := func() error { return nil } + if healcomponents.IsComponentResolutionDisabled() { + return noop, false, nil + } + if command == "install" && isSinglePackageInstall(npmArgs) { + return noop, false, nil + } + resolverRepo, err := ca.resolverRepoForResolution(command) + if err != nil || resolverRepo == "" { + log.Debug("Xray component healing skipped: could not determine resolver repo: " + err.Error()) + return noop, false, nil + } + var projectKey string + if ca.buildConfiguration != nil { + projectKey = ca.buildConfiguration.GetProject() + } + xrayManager, err := xray.CreateXrayServiceManager(ca.serverDetails, xray.WithScopedProjectKey(projectKey)) + if err != nil { + log.Debug("Xray component healing skipped: could not create Xray service manager: " + err.Error()) + return noop, false, nil + } + return healcomponents.RunIfEnabled(ctx, xrayManager, resolverRepo, NewNpmBuildTool(), command, workingDir, ca.npmBootstrapRunner(), extractWorkspaceBootstrapArgs(npmArgs)...) +} + +// resolverRepoForResolution returns the Artifactory virtual repo for dependency policy scope. +func (ca *CommonArgs) resolverRepoForResolution(command string) (string, error) { + if command != "publish" && ca.repo != "" { + return ca.repo, nil + } + if ca.configFilePath != "" { + vConfig, err := project.ReadConfigFile(ca.configFilePath, project.YAML) + if err != nil { + return "", fmt.Errorf("failed to read config file: %w", err) + } + resolverConfig, err := project.GetRepoConfigByPrefix(ca.configFilePath, project.ProjectConfigResolverPrefix, vConfig) + if err != nil { + return "", fmt.Errorf("failed to get resolver config: %w", err) + } + return resolverConfig.TargetRepo(), nil + } + if ca.executablePath != "" { + registryURL, err := ca.getNpmRegistryURL() + if err != nil { + return "", fmt.Errorf("failed to get registry URL: %w", err) + } + return extractRepoName(registryURL) + } + return ca.repo, nil +} + +func (ca *CommonArgs) getNpmRegistryURL() (string, error) { + configCommand := gofrogcmd.Command{ + Executable: ca.executablePath, + CmdName: "config", + CmdArgs: []string{"get", "registry"}, + } + data, err := configCommand.RunWithOutput() + if err != nil { + return "", err + } + return strings.TrimSpace(string(data)), nil +} + +func (ca *CommonArgs) npmBootstrapRunner() healcomponents.CommandRunner { + return func(ctx context.Context, projectRoot string, args ...string) error { + return runNpmAt(ctx, ca.executablePath, projectRoot, args...) + } +} + +func runNpmAt(_ context.Context, executablePath, projectRoot string, args ...string) error { + if len(args) == 0 { + return nil + } + cmd := gofrogcmd.NewCommand(executablePath, args[0], args[1:]) + cmd.Dir = projectRoot + _, err := cmd.RunWithOutput() + return err +} + +func extractWorkspaceBootstrapArgs(npmArgs []string) []string { + var bootstrapArgs []string + for i := 0; i < len(npmArgs); i++ { + arg := npmArgs[i] + if arg == "--workspaces" || arg == "-w" { + bootstrapArgs = append(bootstrapArgs, arg) + continue + } + if strings.HasPrefix(arg, "--workspace=") { + bootstrapArgs = append(bootstrapArgs, arg) + continue + } + if arg == "--workspace" && i+1 < len(npmArgs) { + bootstrapArgs = append(bootstrapArgs, arg, npmArgs[i+1]) + i++ + } + } + return bootstrapArgs +} + +func isSinglePackageInstall(npmArgs []string) bool { + for _, arg := range npmArgs { + if !strings.HasPrefix(arg, "-") { + return true + } + } + return false +} + + +type NpmBuildTool struct{} + +func NewNpmBuildTool() NpmBuildTool { + return NpmBuildTool{} +} + +func (NpmBuildTool) ToolName() string { + return toolName +} + +func (NpmBuildTool) RelevantCommands() []string { + return []string{"install", "ci", "publish"} +} + +func (NpmBuildTool) ProjectRoot(workingDir string) (string, error) { + return discoverProjectRoot(workingDir) +} + +func (t NpmBuildTool) EnsureLockfiles(ctx context.Context, projectRoot, command string, runner healcomponents.CommandRunner, bootstrapArgs ...string) ([]string, error) { + lockPath := filepath.Join(projectRoot, lockfileName) + if _, err := os.Stat(lockPath); err == nil { + return nil, nil + } else if !os.IsNotExist(err) { + return nil, errorutils.CheckError(err) + } + // Lockfile is missing, we need to bootstrap it + switch command { + case "ci": + return nil, errorutils.CheckErrorf("component resolution requires %s for npm ci (generate with npm install first)", lockfileName) + case "install", "publish": + if runner == nil { + return nil, errorutils.CheckErrorf("npm runner required to bootstrap %s", lockfileName) + } + log.Info("Component resolution: generating ", lockfileName, " (lockfile was missing)") + args := append([]string{"install", "--package-lock-only"}, bootstrapArgs...) + if err := runner(ctx, projectRoot, args...); err != nil { + return nil, errorutils.CheckError(err) + } + return []string{lockfileName}, nil + default: + return nil, nil + } +} + +func (NpmBuildTool) DiscoverLockfiles(workingDir string) ([]healcomponents.Lockfile, error) { + root, err := discoverProjectRoot(workingDir) + if err != nil { + return nil, err + } + path := filepath.Join(root, lockfileName) + data, err := os.ReadFile(path) + if os.IsNotExist(err) { + return nil, errorutils.CheckErrorf("expected %s under %s after EnsureLockfiles", lockfileName, root) + } + if err != nil { + return nil, errorutils.CheckError(err) + } + return []healcomponents.Lockfile{{Path: lockfileName, Content: data}}, nil +} + + +// discoverProjectRoot walks up from workingDir to find the npm workspace or lockfile root. +func discoverProjectRoot(workingDir string) (string, error) { + abs, err := filepath.Abs(workingDir) + if err != nil { + return "", errorutils.CheckError(err) + } + + dir := abs + var firstPackageJSON string + for { + pkgPath := filepath.Join(dir, "package.json") + if _, statErr := os.Stat(pkgPath); statErr == nil { + if firstPackageJSON == "" { + firstPackageJSON = dir + } + data, readErr := os.ReadFile(pkgPath) + if readErr != nil { + return "", errorutils.CheckError(readErr) + } + var pkg struct { + Workspaces any `json:"workspaces"` + } + if json.Unmarshal(data, &pkg) == nil && pkg.Workspaces != nil { + return dir, nil + } + } + + lockPath := filepath.Join(dir, "package-lock.json") + if _, statErr := os.Stat(lockPath); statErr == nil { + return dir, nil + } + + parent := filepath.Dir(dir) + if parent == dir { + break + } + dir = parent + } + + if firstPackageJSON != "" { + return firstPackageJSON, nil + } + return abs, nil +} diff --git a/artifactory/commands/npm/xrayheal_test.go b/artifactory/commands/npm/xrayheal_test.go new file mode 100644 index 00000000..828f5f25 --- /dev/null +++ b/artifactory/commands/npm/xrayheal_test.go @@ -0,0 +1,132 @@ +package npm + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/jfrog/jfrog-cli-artifactory/artifactory/healcomponents" +) + + +func TestRunComponentResolution_RespectsDisabledEnv(t *testing.T) { + t.Setenv(healcomponents.HealComponentsDisabledEnvVar, "true") + ca := &CommonArgs{} + ca.SetRepo("npm-virtual").SetServerDetails(nil) + _, healed, err := ca.runXrayComponentHealing(t.Context(), "install", t.TempDir(), nil) + assert.NoError(t, err) + assert.False(t, healed) +} + +func TestEffectiveNpmCommandAfterHeal(t *testing.T) { + nc := &NpmCommand{cmdName: "install", healedLockfile: true} + assert.Equal(t, "ci", nc.effectiveNpmCommand()) + + nc.healedLockfile = false + assert.Equal(t, "install", nc.effectiveNpmCommand()) + + nc.cmdName = "ci" + nc.healedLockfile = true + assert.Equal(t, "ci", nc.effectiveNpmCommand()) +} + +func TestIsSinglePackageInstall(t *testing.T) { + assert.True(t, isSinglePackageInstall([]string{"lodash"})) + assert.True(t, isSinglePackageInstall([]string{"--save", "lodash"})) + assert.False(t, isSinglePackageInstall([]string{"--verbose"})) + assert.False(t, isSinglePackageInstall(nil)) +} + + +func TestNpmBuildTool_DiscoverLockfiles_SingleRootLock(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte(`{"lock":true}`), 0644)) + + tool := NewNpmBuildTool() + files, err := tool.DiscoverLockfiles(dir) + require.NoError(t, err) + require.Len(t, files, 1) + assert.Equal(t, "package-lock.json", files[0].Path) + assert.JSONEq(t, `{"lock":true}`, string(files[0].Content)) +} + +func TestDiscoverProjectRoot_FromWorkspacePackage(t *testing.T) { + root := t.TempDir() + pkgDir := filepath.Join(root, "packages", "app") + require.NoError(t, os.MkdirAll(pkgDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(root, "package.json"), []byte(`{"workspaces":["packages/*"]}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(root, "package-lock.json"), []byte(`{}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(pkgDir, "package.json"), []byte(`{"name":"app"}`), 0644)) + + got, err := discoverProjectRoot(pkgDir) + require.NoError(t, err) + assert.Equal(t, root, got) +} + +func TestNpmBuildTool_EnsureLockfiles_CiRequiresExistingLock(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0644)) + + tool := NewNpmBuildTool() + _, err := tool.EnsureLockfiles(context.Background(), dir, "ci", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "package-lock.json") +} + +func TestNpmBuildTool_EnsureLockfiles_InstallBootstrapsWhenMissing(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0644)) + + var ran []string + runner := func(_ context.Context, root string, args ...string) error { + ran = append(ran, root+":"+strings.Join(args, " ")) + require.NoError(t, os.WriteFile(filepath.Join(root, "package-lock.json"), []byte(`{"bootstrapped":true}`), 0644)) + return nil + } + + tool := NewNpmBuildTool() + _, err := tool.EnsureLockfiles(context.Background(), dir, "install", runner) + require.NoError(t, err) + assert.Contains(t, ran[0], "install --package-lock-only") + _, err = os.Stat(filepath.Join(dir, "package-lock.json")) + require.NoError(t, err) +} + +func TestNpmBuildTool_EnsureLockfiles_SkipsBootstrapWhenLockExists(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte(`{}`), 0644)) + + called := false + runner := func(context.Context, string, ...string) error { + called = true + return nil + } + + tool := NewNpmBuildTool() + _, err := tool.EnsureLockfiles(context.Background(), dir, "install", runner) + require.NoError(t, err) + assert.False(t, called) +} + +func TestNpmBuildTool_EnsureLockfiles_PassesWorkspaceFlags(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0644)) + + var ran []string + runner := func(_ context.Context, root string, args ...string) error { + ran = append(ran, strings.Join(args, " ")) + require.NoError(t, os.WriteFile(filepath.Join(root, "package-lock.json"), []byte(`{}`), 0644)) + return nil + } + + tool := NewNpmBuildTool() + _, err := tool.EnsureLockfiles(context.Background(), dir, "publish", runner, "--workspaces") + require.NoError(t, err) + assert.Contains(t, ran[0], "--workspaces") +} diff --git a/artifactory/healcomponents/service.go b/artifactory/healcomponents/service.go index 81a8d0eb..0bc6dbf8 100644 --- a/artifactory/healcomponents/service.go +++ b/artifactory/healcomponents/service.go @@ -32,25 +32,25 @@ type ComponentResolutionClient interface { } // RunIfEnabled ensures lockfiles exist, discovers them, calls Xray once per file, writes healed lockfiles when changes returned. -// Returns a restore function to revert lockfile writes if the subsequent build-tool command fails. -func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo string, tool BuildTool, command, workingDir string, runner CommandRunner, bootstrapArgs ...string) (restore func() error, err error) { +// Returns a restore function to revert lockfile writes if the subsequent build-tool command fails, and healed=true when at least one lockfile was updated. +func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo string, tool BuildTool, command, workingDir string, runner CommandRunner, bootstrapArgs ...string) (restore func() error, healed bool, err error) { if IsComponentResolutionDisabled() || !IsRelevantCommand(tool, command) { log.Debug("Xray heal components disabled or not relevant command: ", command) - return noopRestore, nil + return noopRestore, false, nil } log.Debug("Running Xray heal components at '" + repo + "' RT repository for tool:", tool.ToolName()) projectRoot, err := tool.ProjectRoot(workingDir) if err != nil { - return noopRestore, err + return noopRestore, false, err } log.Debug("Ensuring lockfiles in project root: ", projectRoot) bootstrapped, err := tool.EnsureLockfiles(ctx, projectRoot, command, runner, bootstrapArgs...) if err != nil { - return noopRestore, err + return noopRestore, false, err } lockfiles, err := tool.DiscoverLockfiles(workingDir) if err != nil { - return noopRestore, err + return noopRestore, false, err } log.Debug("Discovered lockfiles: ", getLockfilePaths(lockfiles)) var toWrite []Lockfile @@ -62,7 +62,7 @@ func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo st Lockfile: json.RawMessage(lf.Content), }) if err != nil { - return noopRestore, errorutils.CheckError(err) + return noopRestore, false, errorutils.CheckError(err) } if len(resp.Changes) == 0 { log.Debug("No changes for ", lf.Path) @@ -76,15 +76,15 @@ func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo st } } if len(toWrite) == 0 { - return noopRestore, nil + return noopRestore, false, nil } log.Debug("Applying", len(toWrite), "healed lockfile(s)...") restore, err = ApplyLockfiles(projectRoot, toWrite, bootstrapped) if err != nil { - return noopRestore, err + return noopRestore, false, err } log.Info("Xray component resolution healed ", totalChanges, " package change(s) across ", len(toWrite), " lockfile(s)") - return restore, nil + return restore, true, nil } func getLockfilePaths(lockfiles []Lockfile) []string { diff --git a/artifactory/healcomponents/service_test.go b/artifactory/healcomponents/service_test.go index 50753cff..57bb3444 100644 --- a/artifactory/healcomponents/service_test.go +++ b/artifactory/healcomponents/service_test.go @@ -49,15 +49,15 @@ func (m mockTool) DiscoverLockfiles(_ string) ([]Lockfile, error) { func TestIsComponentResolutionDisabled(t *testing.T) { t.Run("enabled by default", func(t *testing.T) { - t.Setenv(ComponentResolutionDisabledEnvVar, "") + t.Setenv(HealComponentsDisabledEnvVar, "") assert.False(t, IsComponentResolutionDisabled()) }) t.Run("disabled when env true", func(t *testing.T) { - t.Setenv(ComponentResolutionDisabledEnvVar, "true") + t.Setenv(HealComponentsDisabledEnvVar, "true") assert.True(t, IsComponentResolutionDisabled()) }) t.Run("not disabled for other values", func(t *testing.T) { - t.Setenv(ComponentResolutionDisabledEnvVar, "false") + t.Setenv(HealComponentsDisabledEnvVar, "false") assert.False(t, IsComponentResolutionDisabled()) }) } @@ -73,8 +73,9 @@ func TestRunIfEnabled_WritesHealedLockfiles(t *testing.T) { }} tool := mockTool{root: dir, lockfiles: []Lockfile{{Path: "package-lock.json", Content: []byte("orig")}}} - _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", dir, nil) + _, healed, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", dir, nil) require.NoError(t, err) + assert.True(t, healed) assert.Equal(t, 1, client.callCount) assert.Equal(t, "npm", client.lastReq.BuildTool) assert.Equal(t, "npm-virtual", client.lastReq.Repo) @@ -85,11 +86,12 @@ func TestRunIfEnabled_WritesHealedLockfiles(t *testing.T) { } func TestRunIfEnabled_SkipsWhenDisabled(t *testing.T) { - t.Setenv(ComponentResolutionDisabledEnvVar, "true") + t.Setenv(HealComponentsDisabledEnvVar, "true") client := &mockClient{} tool := mockTool{} - _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", t.TempDir(), nil) + _, healed, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", t.TempDir(), nil) require.NoError(t, err) + assert.False(t, healed) assert.Equal(t, 0, client.callCount) } @@ -104,8 +106,9 @@ func TestRunIfEnabled_SkipsWhenNoChanges(t *testing.T) { }} tool := mockTool{root: dir, lockfiles: []Lockfile{{Path: "package-lock.json", Content: []byte("orig")}}} - _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", dir, nil) + _, healed, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", dir, nil) require.NoError(t, err) + assert.False(t, healed) data, err := os.ReadFile(lockPath) require.NoError(t, err) @@ -122,7 +125,7 @@ func TestRunIfEnabled_LoopsPerDiscoveredFile(t *testing.T) { {Path: "a.lock", Content: []byte("a")}, {Path: "b.lock", Content: []byte("b")}, }} - _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", dir, nil) + _, _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", dir, nil) require.NoError(t, err) assert.Equal(t, 2, client.callCount) } @@ -130,7 +133,7 @@ func TestRunIfEnabled_LoopsPerDiscoveredFile(t *testing.T) { func TestRunIfEnabled_SkipsIrrelevantCommand(t *testing.T) { client := &mockClient{} tool := mockTool{} - _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "version", t.TempDir(), nil) + _, _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "version", t.TempDir(), nil) require.NoError(t, err) assert.Equal(t, 0, client.callCount) } @@ -138,7 +141,7 @@ func TestRunIfEnabled_SkipsIrrelevantCommand(t *testing.T) { func TestRunIfEnabled_PropagatesEnsureLockfilesError(t *testing.T) { client := &mockClient{} tool := mockTool{ensureErr: errors.New("package-lock.json is required for npm ci")} - _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "ci", t.TempDir(), nil) + _, _, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "ci", t.TempDir(), nil) require.Error(t, err) assert.Contains(t, err.Error(), "package-lock.json") assert.Equal(t, 0, client.callCount) diff --git a/go.mod b/go.mod index 6e44008d..0611fa56 100644 --- a/go.mod +++ b/go.mod @@ -203,10 +203,10 @@ require ( // replace github.com/jfrog/jfrog-cli-core/v2 => /Users/assafa/code/jfrog/jfrog-cli-core // attiasas:xray_component_resolution -replace github.com/jfrog/jfrog-client-go => github.com/attiasas/jfrog-client-go v0.0.0-20260617113537-5536e6ba8c42 +replace github.com/jfrog/jfrog-client-go => github.com/attiasas/jfrog-client-go v0.0.0-20260617154718-b93ae683ce3a // attiasas:expend_xray_manager -replace github.com/jfrog/jfrog-cli-core/v2 => github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617114425-0d9f6f50df64 +replace github.com/jfrog/jfrog-cli-core/v2 => github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617154835-a95b1c993846 // replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260604085947-7c110b77b4b4 diff --git a/go.sum b/go.sum index 22ef0cd0..c7438f6b 100644 --- a/go.sum +++ b/go.sum @@ -59,10 +59,10 @@ github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPd github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= -github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617114425-0d9f6f50df64 h1:b+3Io6f60cku95RoJOi0N0fmHWpcVq+nUgLtn9jNmmk= -github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617114425-0d9f6f50df64/go.mod h1:MZApLFA/8EyIA9BTGCDUw412bWZ4DXSfmqxwUWlparo= -github.com/attiasas/jfrog-client-go v0.0.0-20260617113537-5536e6ba8c42 h1:59+NXeZ5raJXnU/hQZ5lv36ry8D2wFGpfc8eXiAwhLE= -github.com/attiasas/jfrog-client-go v0.0.0-20260617113537-5536e6ba8c42/go.mod h1:FHpjN1nTDoj96xd6obe27EOgGErqzU0rQgC96L3Ch9E= +github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617154835-a95b1c993846 h1:2Yab01ypFZPQvkVIuOCv/rmU6zCSnNdFI2eJsdw2ZvI= +github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617154835-a95b1c993846/go.mod h1:aEid1049BYVbG0mwPlI4TWvX2q+K1El5N8MiDsSbAPg= +github.com/attiasas/jfrog-client-go v0.0.0-20260617154718-b93ae683ce3a h1:kPPP6Mex+tusTw/c6XczrxFXHQOISm/BZ+oEtsl0LP0= +github.com/attiasas/jfrog-client-go v0.0.0-20260617154718-b93ae683ce3a/go.mod h1:FHpjN1nTDoj96xd6obe27EOgGErqzU0rQgC96L3Ch9E= github.com/aws/aws-sdk-go v1.55.8 h1:JRmEUbU52aJQZ2AjX4q4Wu7t4uZjOu71uyNmaWlUkJQ= github.com/aws/aws-sdk-go v1.55.8/go.mod h1:ZkViS9AqA6otK+JBBNH2++sx1sgxrPKcSzPPvQkUtXk= github.com/aws/aws-sdk-go-v2 v1.41.1 h1:ABlyEARCDLN034NhxlRUSZr4l71mh+T5KAeGh6cerhU= From 97fe83a67963e93e959951e7de732d471809356b Mon Sep 17 00:00:00 2001 From: attiasas Date: Wed, 17 Jun 2026 19:00:33 +0300 Subject: [PATCH 4/9] format --- artifactory/commands/npm/xrayheal.go | 8 +++----- artifactory/commands/npm/xrayheal_test.go | 2 -- artifactory/healcomponents/service.go | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/artifactory/commands/npm/xrayheal.go b/artifactory/commands/npm/xrayheal.go index b04d2bd0..d86d177a 100644 --- a/artifactory/commands/npm/xrayheal.go +++ b/artifactory/commands/npm/xrayheal.go @@ -3,10 +3,10 @@ package npm import ( "context" "encoding/json" - "strings" - "os" "fmt" + "os" "path/filepath" + "strings" "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/jfrog/jfrog-client-go/utils/log" @@ -20,7 +20,7 @@ import ( ) const ( - toolName = "npm" + toolName = "npm" lockfileName = "package-lock.json" ) @@ -133,7 +133,6 @@ func isSinglePackageInstall(npmArgs []string) bool { return false } - type NpmBuildTool struct{} func NewNpmBuildTool() NpmBuildTool { @@ -194,7 +193,6 @@ func (NpmBuildTool) DiscoverLockfiles(workingDir string) ([]healcomponents.Lockf return []healcomponents.Lockfile{{Path: lockfileName, Content: data}}, nil } - // discoverProjectRoot walks up from workingDir to find the npm workspace or lockfile root. func discoverProjectRoot(workingDir string) (string, error) { abs, err := filepath.Abs(workingDir) diff --git a/artifactory/commands/npm/xrayheal_test.go b/artifactory/commands/npm/xrayheal_test.go index 828f5f25..e8cdfeec 100644 --- a/artifactory/commands/npm/xrayheal_test.go +++ b/artifactory/commands/npm/xrayheal_test.go @@ -13,7 +13,6 @@ import ( "github.com/jfrog/jfrog-cli-artifactory/artifactory/healcomponents" ) - func TestRunComponentResolution_RespectsDisabledEnv(t *testing.T) { t.Setenv(healcomponents.HealComponentsDisabledEnvVar, "true") ca := &CommonArgs{} @@ -42,7 +41,6 @@ func TestIsSinglePackageInstall(t *testing.T) { assert.False(t, isSinglePackageInstall(nil)) } - func TestNpmBuildTool_DiscoverLockfiles_SingleRootLock(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte(`{"lock":true}`), 0644)) diff --git a/artifactory/healcomponents/service.go b/artifactory/healcomponents/service.go index 0bc6dbf8..5485ee98 100644 --- a/artifactory/healcomponents/service.go +++ b/artifactory/healcomponents/service.go @@ -38,7 +38,7 @@ func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo st log.Debug("Xray heal components disabled or not relevant command: ", command) return noopRestore, false, nil } - log.Debug("Running Xray heal components at '" + repo + "' RT repository for tool:", tool.ToolName()) + log.Debug("Running Xray heal components at '"+repo+"' RT repository for tool:", tool.ToolName()) projectRoot, err := tool.ProjectRoot(workingDir) if err != nil { return noopRestore, false, err From ef7de242babf01e539ea41427443549792daa32f Mon Sep 17 00:00:00 2001 From: attiasas Date: Wed, 17 Jun 2026 21:51:01 +0300 Subject: [PATCH 5/9] update lockfile to string --- artifactory/healcomponents/service.go | 5 +- artifactory/healcomponents/service_test.go | 53 ++++++++++++++++++---- go.mod | 4 +- go.sum | 8 ++-- 4 files changed, 52 insertions(+), 18 deletions(-) diff --git a/artifactory/healcomponents/service.go b/artifactory/healcomponents/service.go index 5485ee98..30eac495 100644 --- a/artifactory/healcomponents/service.go +++ b/artifactory/healcomponents/service.go @@ -2,7 +2,6 @@ package healcomponents import ( "context" - "encoding/json" "os" "github.com/jfrog/jfrog-client-go/utils/errorutils" @@ -59,7 +58,7 @@ func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo st resp, err := client.HealComponents(services.ComponentResolutionRequest{ BuildTool: tool.ToolName(), Repo: repo, - Lockfile: json.RawMessage(lf.Content), + Lockfile: string(lf.Content), }) if err != nil { return noopRestore, false, errorutils.CheckError(err) @@ -68,7 +67,7 @@ func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo st log.Debug("No changes for ", lf.Path) continue } - toWrite = append(toWrite, Lockfile{Path: lf.Path, Content: resp.Content}) + toWrite = append(toWrite, Lockfile{Path: lf.Path, Content: []byte(resp.Lockfile)}) totalChanges += len(resp.Changes) log.Debug("Healed ", lf.Path, " with ", len(resp.Changes), " package change(s)") for _, ch := range resp.Changes { diff --git a/artifactory/healcomponents/service_test.go b/artifactory/healcomponents/service_test.go index 57bb3444..fb4a320e 100644 --- a/artifactory/healcomponents/service_test.go +++ b/artifactory/healcomponents/service_test.go @@ -2,7 +2,6 @@ package healcomponents import ( "context" - "encoding/json" "errors" "os" "path/filepath" @@ -27,13 +26,25 @@ func (m *mockClient) HealComponents(req services.ComponentResolutionRequest) (*s } type mockTool struct { + name string + commands []string root string lockfiles []Lockfile ensureErr error } -func (m mockTool) ToolName() string { return "npm" } -func (m mockTool) RelevantCommands() []string { return []string{"install", "ci"} } +func (m mockTool) ToolName() string { + if m.name != "" { + return m.name + } + return "npm" +} +func (m mockTool) RelevantCommands() []string { + if len(m.commands) > 0 { + return m.commands + } + return []string{"install", "ci"} +} func (m mockTool) ProjectRoot(_ string) (string, error) { return m.root, nil } @@ -68,8 +79,8 @@ func TestRunIfEnabled_WritesHealedLockfiles(t *testing.T) { require.NoError(t, os.WriteFile(lockPath, []byte("orig"), 0644)) client := &mockClient{resp: services.ComponentResolutionResponse{ - Content: json.RawMessage(`"healed"`), - Changes: []services.Change{{Package: "lodash", BeforeIntegrity: "a", AfterIntegrity: "b"}}, + Lockfile: "healed", + Changes: []services.Change{{Package: "lodash", BeforeIntegrity: "a", AfterIntegrity: "b"}}, }} tool := mockTool{root: dir, lockfiles: []Lockfile{{Path: "package-lock.json", Content: []byte("orig")}}} @@ -79,10 +90,34 @@ func TestRunIfEnabled_WritesHealedLockfiles(t *testing.T) { assert.Equal(t, 1, client.callCount) assert.Equal(t, "npm", client.lastReq.BuildTool) assert.Equal(t, "npm-virtual", client.lastReq.Repo) + assert.Equal(t, "orig", client.lastReq.Lockfile) + + data, err := os.ReadFile(lockPath) + require.NoError(t, err) + assert.Equal(t, "healed", string(data)) +} + +func TestRunIfEnabled_WritesHealedNpmLockAsString(t *testing.T) { + dir := t.TempDir() + lockPath := filepath.Join(dir, "package-lock.json") + orig := `{"lockfileVersion":3,"name":"app"}` + require.NoError(t, os.WriteFile(lockPath, []byte(orig), 0644)) + + healed := `{"lockfileVersion":3,"name":"app","healed":true}` + client := &mockClient{resp: services.ComponentResolutionResponse{ + Lockfile: healed, + Changes: []services.Change{{Package: "lodash", BeforeIntegrity: "a", AfterIntegrity: "b"}}, + }} + tool := mockTool{root: dir, lockfiles: []Lockfile{{Path: "package-lock.json", Content: []byte(orig)}}} + + _, healedFlag, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", dir, nil) + require.NoError(t, err) + assert.True(t, healedFlag) data, err := os.ReadFile(lockPath) require.NoError(t, err) - assert.Equal(t, `"healed"`, string(data)) + assert.JSONEq(t, healed, string(data)) + assert.Equal(t, orig, client.lastReq.Lockfile) } func TestRunIfEnabled_SkipsWhenDisabled(t *testing.T) { @@ -101,8 +136,8 @@ func TestRunIfEnabled_SkipsWhenNoChanges(t *testing.T) { require.NoError(t, os.WriteFile(lockPath, []byte("orig"), 0644)) client := &mockClient{resp: services.ComponentResolutionResponse{ - Content: json.RawMessage(`"orig"`), - Changes: nil, + Lockfile: "orig", + Changes: nil, }} tool := mockTool{root: dir, lockfiles: []Lockfile{{Path: "package-lock.json", Content: []byte("orig")}}} @@ -120,7 +155,7 @@ func TestRunIfEnabled_LoopsPerDiscoveredFile(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dir, "a.lock"), []byte("a"), 0644)) require.NoError(t, os.WriteFile(filepath.Join(dir, "b.lock"), []byte("b"), 0644)) - client := &mockClient{resp: services.ComponentResolutionResponse{Content: json.RawMessage(`"x"`), Changes: nil}} + client := &mockClient{resp: services.ComponentResolutionResponse{Lockfile: "x", Changes: nil}} tool := mockTool{root: dir, lockfiles: []Lockfile{ {Path: "a.lock", Content: []byte("a")}, {Path: "b.lock", Content: []byte("b")}, diff --git a/go.mod b/go.mod index 0611fa56..9431b91c 100644 --- a/go.mod +++ b/go.mod @@ -203,10 +203,10 @@ require ( // replace github.com/jfrog/jfrog-cli-core/v2 => /Users/assafa/code/jfrog/jfrog-cli-core // attiasas:xray_component_resolution -replace github.com/jfrog/jfrog-client-go => github.com/attiasas/jfrog-client-go v0.0.0-20260617154718-b93ae683ce3a +replace github.com/jfrog/jfrog-client-go => github.com/attiasas/jfrog-client-go v0.0.0-20260617184739-f8431de360c6 // attiasas:expend_xray_manager -replace github.com/jfrog/jfrog-cli-core/v2 => github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617154835-a95b1c993846 +replace github.com/jfrog/jfrog-cli-core/v2 => github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617184845-0cfaec9c03c7 // replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260604085947-7c110b77b4b4 diff --git a/go.sum b/go.sum index c7438f6b..c238a448 100644 --- a/go.sum +++ b/go.sum @@ -59,10 +59,10 @@ github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPd github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= -github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617154835-a95b1c993846 h1:2Yab01ypFZPQvkVIuOCv/rmU6zCSnNdFI2eJsdw2ZvI= -github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617154835-a95b1c993846/go.mod h1:aEid1049BYVbG0mwPlI4TWvX2q+K1El5N8MiDsSbAPg= -github.com/attiasas/jfrog-client-go v0.0.0-20260617154718-b93ae683ce3a h1:kPPP6Mex+tusTw/c6XczrxFXHQOISm/BZ+oEtsl0LP0= -github.com/attiasas/jfrog-client-go v0.0.0-20260617154718-b93ae683ce3a/go.mod h1:FHpjN1nTDoj96xd6obe27EOgGErqzU0rQgC96L3Ch9E= +github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617184845-0cfaec9c03c7 h1:T7V2yJJFyKh+cf1thzTuAZThD/nXDH3GDHNCFyOzM/s= +github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617184845-0cfaec9c03c7/go.mod h1:Tf6NEshuw3NShJabLP1/FHuATGBhWKW/6WjETi0LhKc= +github.com/attiasas/jfrog-client-go v0.0.0-20260617184739-f8431de360c6 h1:OwS+TQYZvx+PiZgT043h7nagc5yYG410HG8JZGPP9FU= +github.com/attiasas/jfrog-client-go v0.0.0-20260617184739-f8431de360c6/go.mod h1:FHpjN1nTDoj96xd6obe27EOgGErqzU0rQgC96L3Ch9E= github.com/aws/aws-sdk-go v1.55.8 h1:JRmEUbU52aJQZ2AjX4q4Wu7t4uZjOu71uyNmaWlUkJQ= github.com/aws/aws-sdk-go v1.55.8/go.mod h1:ZkViS9AqA6otK+JBBNH2++sx1sgxrPKcSzPPvQkUtXk= github.com/aws/aws-sdk-go-v2 v1.41.1 h1:ABlyEARCDLN034NhxlRUSZr4l71mh+T5KAeGh6cerhU= From 16458ab04c2fdbc3ddd95bb3b2c161d6f1dd0a3c Mon Sep 17 00:00:00 2001 From: attiasas Date: Thu, 18 Jun 2026 09:35:33 +0300 Subject: [PATCH 6/9] refactor code --- artifactory/commands/npm/publish.go | 2 +- artifactory/commands/npm/xrayheal.go | 154 ++---------------- artifactory/commands/npm/xrayheal_test.go | 93 ----------- artifactory/healcomponents/files.go | 63 ------- artifactory/healcomponents/files_test.go | 32 ---- artifactory/healcomponents/npm/args_parse.go | 76 +++++++++ .../healcomponents/npm/args_parse_test.go | 43 +++++ artifactory/healcomponents/npm/discover.go | 68 ++++++++ .../healcomponents/npm/discover_test.go | 92 +++++++++++ artifactory/healcomponents/npm/npm.go | 91 +++++++++++ artifactory/healcomponents/npm/npm_test.go | 125 ++++++++++++++ .../healcomponents/npm/package_json.go | 35 ++++ .../healcomponents/npm/package_json_test.go | 32 ++++ artifactory/healcomponents/service.go | 62 ++++++- artifactory/healcomponents/service_test.go | 22 +++ 15 files changed, 658 insertions(+), 332 deletions(-) delete mode 100644 artifactory/healcomponents/files.go delete mode 100644 artifactory/healcomponents/files_test.go create mode 100644 artifactory/healcomponents/npm/args_parse.go create mode 100644 artifactory/healcomponents/npm/args_parse_test.go create mode 100644 artifactory/healcomponents/npm/discover.go create mode 100644 artifactory/healcomponents/npm/discover_test.go create mode 100644 artifactory/healcomponents/npm/npm.go create mode 100644 artifactory/healcomponents/npm/npm_test.go create mode 100644 artifactory/healcomponents/npm/package_json.go create mode 100644 artifactory/healcomponents/npm/package_json_test.go diff --git a/artifactory/commands/npm/publish.go b/artifactory/commands/npm/publish.go index 2b3f9895..b54e6acd 100644 --- a/artifactory/commands/npm/publish.go +++ b/artifactory/commands/npm/publish.go @@ -174,7 +174,7 @@ func (npc *NpmPublishCommand) Run() (err error) { return err } var restoreResolution func() error - restoreResolution, _, err = npc.runXrayComponentHealing(context.Background(), "publish", npc.workingDirectory, npc.npmArgs) + restoreResolution, _, err = npc.runXrayComponentHealingForPublish(context.Background(), "publish", npc.workingDirectory, npc.publishPath, npc.npmArgs) if err != nil { return err } diff --git a/artifactory/commands/npm/xrayheal.go b/artifactory/commands/npm/xrayheal.go index d86d177a..d1448a18 100644 --- a/artifactory/commands/npm/xrayheal.go +++ b/artifactory/commands/npm/xrayheal.go @@ -2,14 +2,9 @@ package npm import ( "context" - "encoding/json" "fmt" - "os" - "path/filepath" - "strings" - - "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/jfrog/jfrog-client-go/utils/log" + "strings" gofrogcmd "github.com/jfrog/gofrog/io" @@ -17,21 +12,25 @@ import ( "github.com/jfrog/jfrog-cli-core/v2/utils/xray" "github.com/jfrog/jfrog-cli-artifactory/artifactory/healcomponents" -) - -const ( - toolName = "npm" - lockfileName = "package-lock.json" + cnpm "github.com/jfrog/jfrog-cli-artifactory/artifactory/healcomponents/npm" ) func (ca *CommonArgs) runXrayComponentHealing(ctx context.Context, command, workingDir string, npmArgs []string) (restore func() error, healed bool, err error) { + if command == "install" && isSinglePackageInstall(npmArgs) { + return func() error { return nil }, false, nil + } + return ca.runXrayComponentHealingWithTool(ctx, command, workingDir, cnpm.NewBuildToolWithArgs(npmArgs), cnpm.BootstrapArgsFrom(npmArgs)...) +} + +func (ca *CommonArgs) runXrayComponentHealingForPublish(ctx context.Context, command, workingDir, publishPath string, npmArgs []string) (restore func() error, healed bool, err error) { + return ca.runXrayComponentHealingWithTool(ctx, command, workingDir, cnpm.NewBuildToolForPublish(workingDir, publishPath, npmArgs), cnpm.BootstrapArgsFrom(npmArgs)...) +} + +func (ca *CommonArgs) runXrayComponentHealingWithTool(ctx context.Context, command, workingDir string, tool cnpm.BuildTool, bootstrapArgs ...string) (restore func() error, healed bool, err error) { noop := func() error { return nil } if healcomponents.IsComponentResolutionDisabled() { return noop, false, nil } - if command == "install" && isSinglePackageInstall(npmArgs) { - return noop, false, nil - } resolverRepo, err := ca.resolverRepoForResolution(command) if err != nil || resolverRepo == "" { log.Debug("Xray component healing skipped: could not determine resolver repo: " + err.Error()) @@ -46,7 +45,7 @@ func (ca *CommonArgs) runXrayComponentHealing(ctx context.Context, command, work log.Debug("Xray component healing skipped: could not create Xray service manager: " + err.Error()) return noop, false, nil } - return healcomponents.RunIfEnabled(ctx, xrayManager, resolverRepo, NewNpmBuildTool(), command, workingDir, ca.npmBootstrapRunner(), extractWorkspaceBootstrapArgs(npmArgs)...) + return healcomponents.RunIfEnabled(ctx, xrayManager, resolverRepo, tool, command, workingDir, ca.npmBootstrapRunner(), bootstrapArgs...) } // resolverRepoForResolution returns the Artifactory virtual repo for dependency policy scope. @@ -104,26 +103,6 @@ func runNpmAt(_ context.Context, executablePath, projectRoot string, args ...str return err } -func extractWorkspaceBootstrapArgs(npmArgs []string) []string { - var bootstrapArgs []string - for i := 0; i < len(npmArgs); i++ { - arg := npmArgs[i] - if arg == "--workspaces" || arg == "-w" { - bootstrapArgs = append(bootstrapArgs, arg) - continue - } - if strings.HasPrefix(arg, "--workspace=") { - bootstrapArgs = append(bootstrapArgs, arg) - continue - } - if arg == "--workspace" && i+1 < len(npmArgs) { - bootstrapArgs = append(bootstrapArgs, arg, npmArgs[i+1]) - i++ - } - } - return bootstrapArgs -} - func isSinglePackageInstall(npmArgs []string) bool { for _, arg := range npmArgs { if !strings.HasPrefix(arg, "-") { @@ -132,108 +111,3 @@ func isSinglePackageInstall(npmArgs []string) bool { } return false } - -type NpmBuildTool struct{} - -func NewNpmBuildTool() NpmBuildTool { - return NpmBuildTool{} -} - -func (NpmBuildTool) ToolName() string { - return toolName -} - -func (NpmBuildTool) RelevantCommands() []string { - return []string{"install", "ci", "publish"} -} - -func (NpmBuildTool) ProjectRoot(workingDir string) (string, error) { - return discoverProjectRoot(workingDir) -} - -func (t NpmBuildTool) EnsureLockfiles(ctx context.Context, projectRoot, command string, runner healcomponents.CommandRunner, bootstrapArgs ...string) ([]string, error) { - lockPath := filepath.Join(projectRoot, lockfileName) - if _, err := os.Stat(lockPath); err == nil { - return nil, nil - } else if !os.IsNotExist(err) { - return nil, errorutils.CheckError(err) - } - // Lockfile is missing, we need to bootstrap it - switch command { - case "ci": - return nil, errorutils.CheckErrorf("component resolution requires %s for npm ci (generate with npm install first)", lockfileName) - case "install", "publish": - if runner == nil { - return nil, errorutils.CheckErrorf("npm runner required to bootstrap %s", lockfileName) - } - log.Info("Component resolution: generating ", lockfileName, " (lockfile was missing)") - args := append([]string{"install", "--package-lock-only"}, bootstrapArgs...) - if err := runner(ctx, projectRoot, args...); err != nil { - return nil, errorutils.CheckError(err) - } - return []string{lockfileName}, nil - default: - return nil, nil - } -} - -func (NpmBuildTool) DiscoverLockfiles(workingDir string) ([]healcomponents.Lockfile, error) { - root, err := discoverProjectRoot(workingDir) - if err != nil { - return nil, err - } - path := filepath.Join(root, lockfileName) - data, err := os.ReadFile(path) - if os.IsNotExist(err) { - return nil, errorutils.CheckErrorf("expected %s under %s after EnsureLockfiles", lockfileName, root) - } - if err != nil { - return nil, errorutils.CheckError(err) - } - return []healcomponents.Lockfile{{Path: lockfileName, Content: data}}, nil -} - -// discoverProjectRoot walks up from workingDir to find the npm workspace or lockfile root. -func discoverProjectRoot(workingDir string) (string, error) { - abs, err := filepath.Abs(workingDir) - if err != nil { - return "", errorutils.CheckError(err) - } - - dir := abs - var firstPackageJSON string - for { - pkgPath := filepath.Join(dir, "package.json") - if _, statErr := os.Stat(pkgPath); statErr == nil { - if firstPackageJSON == "" { - firstPackageJSON = dir - } - data, readErr := os.ReadFile(pkgPath) - if readErr != nil { - return "", errorutils.CheckError(readErr) - } - var pkg struct { - Workspaces any `json:"workspaces"` - } - if json.Unmarshal(data, &pkg) == nil && pkg.Workspaces != nil { - return dir, nil - } - } - - lockPath := filepath.Join(dir, "package-lock.json") - if _, statErr := os.Stat(lockPath); statErr == nil { - return dir, nil - } - - parent := filepath.Dir(dir) - if parent == dir { - break - } - dir = parent - } - - if firstPackageJSON != "" { - return firstPackageJSON, nil - } - return abs, nil -} diff --git a/artifactory/commands/npm/xrayheal_test.go b/artifactory/commands/npm/xrayheal_test.go index e8cdfeec..1b5569e0 100644 --- a/artifactory/commands/npm/xrayheal_test.go +++ b/artifactory/commands/npm/xrayheal_test.go @@ -1,14 +1,9 @@ package npm import ( - "context" - "os" - "path/filepath" - "strings" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/jfrog/jfrog-cli-artifactory/artifactory/healcomponents" ) @@ -40,91 +35,3 @@ func TestIsSinglePackageInstall(t *testing.T) { assert.False(t, isSinglePackageInstall([]string{"--verbose"})) assert.False(t, isSinglePackageInstall(nil)) } - -func TestNpmBuildTool_DiscoverLockfiles_SingleRootLock(t *testing.T) { - dir := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte(`{"lock":true}`), 0644)) - - tool := NewNpmBuildTool() - files, err := tool.DiscoverLockfiles(dir) - require.NoError(t, err) - require.Len(t, files, 1) - assert.Equal(t, "package-lock.json", files[0].Path) - assert.JSONEq(t, `{"lock":true}`, string(files[0].Content)) -} - -func TestDiscoverProjectRoot_FromWorkspacePackage(t *testing.T) { - root := t.TempDir() - pkgDir := filepath.Join(root, "packages", "app") - require.NoError(t, os.MkdirAll(pkgDir, 0755)) - require.NoError(t, os.WriteFile(filepath.Join(root, "package.json"), []byte(`{"workspaces":["packages/*"]}`), 0644)) - require.NoError(t, os.WriteFile(filepath.Join(root, "package-lock.json"), []byte(`{}`), 0644)) - require.NoError(t, os.WriteFile(filepath.Join(pkgDir, "package.json"), []byte(`{"name":"app"}`), 0644)) - - got, err := discoverProjectRoot(pkgDir) - require.NoError(t, err) - assert.Equal(t, root, got) -} - -func TestNpmBuildTool_EnsureLockfiles_CiRequiresExistingLock(t *testing.T) { - dir := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0644)) - - tool := NewNpmBuildTool() - _, err := tool.EnsureLockfiles(context.Background(), dir, "ci", nil) - require.Error(t, err) - assert.Contains(t, err.Error(), "package-lock.json") -} - -func TestNpmBuildTool_EnsureLockfiles_InstallBootstrapsWhenMissing(t *testing.T) { - dir := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0644)) - - var ran []string - runner := func(_ context.Context, root string, args ...string) error { - ran = append(ran, root+":"+strings.Join(args, " ")) - require.NoError(t, os.WriteFile(filepath.Join(root, "package-lock.json"), []byte(`{"bootstrapped":true}`), 0644)) - return nil - } - - tool := NewNpmBuildTool() - _, err := tool.EnsureLockfiles(context.Background(), dir, "install", runner) - require.NoError(t, err) - assert.Contains(t, ran[0], "install --package-lock-only") - _, err = os.Stat(filepath.Join(dir, "package-lock.json")) - require.NoError(t, err) -} - -func TestNpmBuildTool_EnsureLockfiles_SkipsBootstrapWhenLockExists(t *testing.T) { - dir := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0644)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte(`{}`), 0644)) - - called := false - runner := func(context.Context, string, ...string) error { - called = true - return nil - } - - tool := NewNpmBuildTool() - _, err := tool.EnsureLockfiles(context.Background(), dir, "install", runner) - require.NoError(t, err) - assert.False(t, called) -} - -func TestNpmBuildTool_EnsureLockfiles_PassesWorkspaceFlags(t *testing.T) { - dir := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0644)) - - var ran []string - runner := func(_ context.Context, root string, args ...string) error { - ran = append(ran, strings.Join(args, " ")) - require.NoError(t, os.WriteFile(filepath.Join(root, "package-lock.json"), []byte(`{}`), 0644)) - return nil - } - - tool := NewNpmBuildTool() - _, err := tool.EnsureLockfiles(context.Background(), dir, "publish", runner, "--workspaces") - require.NoError(t, err) - assert.Contains(t, ran[0], "--workspaces") -} diff --git a/artifactory/healcomponents/files.go b/artifactory/healcomponents/files.go deleted file mode 100644 index 92bad529..00000000 --- a/artifactory/healcomponents/files.go +++ /dev/null @@ -1,63 +0,0 @@ -package healcomponents - -import ( - "os" - "path/filepath" - - "github.com/jfrog/jfrog-client-go/utils/errorutils" -) - -type lockfileBackup struct { - path string - content []byte // nil means the file did not exist before apply -} - -// ApplyLockfiles backs up existing lockfiles under projectRoot, writes updates, returns restore func. -// Paths listed in treatAsAbsent are restored by deletion even if they exist on disk (bootstrapped locks). -func ApplyLockfiles(projectRoot string, lockfiles []Lockfile, treatAsAbsent []string) (restore func() error, err error) { - if len(lockfiles) == 0 { - return func() error { return nil }, nil - } - - absent := make(map[string]bool, len(treatAsAbsent)) - for _, path := range treatAsAbsent { - absent[path] = true - } - - var backups []lockfileBackup - for _, lf := range lockfiles { - fullPath := filepath.Join(projectRoot, lf.Path) - backup := lockfileBackup{path: fullPath} - if !absent[lf.Path] { - data, readErr := os.ReadFile(fullPath) - if readErr == nil { - backup.content = data - } else if !os.IsNotExist(readErr) { - return nil, errorutils.CheckError(readErr) - } - } - backups = append(backups, backup) - - if err = os.MkdirAll(filepath.Dir(fullPath), 0755); err != nil { - return nil, errorutils.CheckError(err) - } - if err = os.WriteFile(fullPath, lf.Content, 0644); err != nil { - return nil, errorutils.CheckError(err) - } - } - - return func() error { - for _, backup := range backups { - if backup.content == nil { - if err = os.Remove(backup.path); err != nil && !os.IsNotExist(err) { - return errorutils.CheckError(err) - } - continue - } - if err = os.WriteFile(backup.path, backup.content, 0644); err != nil { - return errorutils.CheckError(err) - } - } - return nil - }, nil -} diff --git a/artifactory/healcomponents/files_test.go b/artifactory/healcomponents/files_test.go deleted file mode 100644 index d6cd9d51..00000000 --- a/artifactory/healcomponents/files_test.go +++ /dev/null @@ -1,32 +0,0 @@ -package healcomponents - -import ( - "os" - "path/filepath" - "testing" - - testsutils "github.com/jfrog/jfrog-client-go/utils/tests" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestApplyLockfiles_WritesMultipleFiles(t *testing.T) { - dir := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte("a"), 0644)) - require.NoError(t, os.MkdirAll(filepath.Join(dir, "app"), 0755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "app/gradle.lockfile"), []byte("b"), 0644)) - - restore, err := ApplyLockfiles(dir, []Lockfile{ - {Path: "package-lock.json", Content: []byte("a-healed")}, - {Path: "app/gradle.lockfile", Content: []byte("b-healed")}, - }, nil) - require.NoError(t, err) - defer testsutils.RemoveAllAndAssert(t, dir) - - a, _ := os.ReadFile(filepath.Join(dir, "package-lock.json")) - b, _ := os.ReadFile(filepath.Join(dir, "app/gradle.lockfile")) - assert.Equal(t, "a-healed", string(a)) - assert.Equal(t, "b-healed", string(b)) - - require.NoError(t, restore()) -} diff --git a/artifactory/healcomponents/npm/args_parse.go b/artifactory/healcomponents/npm/args_parse.go new file mode 100644 index 00000000..751b8bbd --- /dev/null +++ b/artifactory/healcomponents/npm/args_parse.go @@ -0,0 +1,76 @@ +package npm + +import ( + "path/filepath" + "strings" + + "github.com/jfrog/jfrog-client-go/utils/errorutils" +) + +type discoveryOptions struct { + prefixDir string + publishPath string +} + +type npmCLIArgs struct { + prefixDir string + bootstrapArgs []string +} + +func parseNpmCLIArgs(args []string) npmCLIArgs { + var out npmCLIArgs + for i := 0; i < len(args); i++ { + arg := args[i] + switch { + case arg == "--prefix" || arg == "--cwd" || arg == "-C": + if i+1 < len(args) { + i++ + out.prefixDir = args[i] + } + case strings.HasPrefix(arg, "--prefix="): + out.prefixDir = strings.TrimPrefix(arg, "--prefix=") + case strings.HasPrefix(arg, "--cwd="): + out.prefixDir = strings.TrimPrefix(arg, "--cwd=") + case strings.HasPrefix(arg, "-C"): + if arg == "-C" { + continue + } + out.prefixDir = strings.TrimPrefix(arg, "-C") + case arg == "--workspaces" || arg == "-w": + out.bootstrapArgs = append(out.bootstrapArgs, arg) + case strings.HasPrefix(arg, "--workspace="): + out.bootstrapArgs = append(out.bootstrapArgs, arg) + case arg == "--workspace" && i+1 < len(args): + out.bootstrapArgs = append(out.bootstrapArgs, arg, args[i+1]) + i++ + } + } + return out +} + +// BootstrapArgsFrom extracts workspace flags to pass to npm install --package-lock-only. +func BootstrapArgsFrom(npmArgs []string) []string { + return parseNpmCLIArgs(npmArgs).bootstrapArgs +} + +func effectiveStartDir(workingDir string, opts discoveryOptions) (string, error) { + abs, err := filepath.Abs(workingDir) + if err != nil { + return "", errorutils.CheckError(err) + } + if opts.publishPath != "" { + p := opts.publishPath + if !filepath.IsAbs(p) { + p = filepath.Join(abs, p) + } + return filepath.Clean(p), nil + } + if opts.prefixDir != "" { + p := opts.prefixDir + if !filepath.IsAbs(p) { + p = filepath.Join(abs, p) + } + return filepath.Clean(p), nil + } + return abs, nil +} diff --git a/artifactory/healcomponents/npm/args_parse_test.go b/artifactory/healcomponents/npm/args_parse_test.go new file mode 100644 index 00000000..ad76a4c3 --- /dev/null +++ b/artifactory/healcomponents/npm/args_parse_test.go @@ -0,0 +1,43 @@ +package npm + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseNpmCLIArgs_Prefix(t *testing.T) { + opts := parseNpmCLIArgs([]string{"install", "--prefix", "sub/pkg"}) + assert.Equal(t, "sub/pkg", opts.prefixDir) +} + +func TestParseNpmCLIArgs_CShort(t *testing.T) { + opts := parseNpmCLIArgs([]string{"-C", "services/api", "ci"}) + assert.Equal(t, "services/api", opts.prefixDir) +} + +func TestParseNpmCLIArgs_PrefixEquals(t *testing.T) { + opts := parseNpmCLIArgs([]string{"install", "--prefix=frontend"}) + assert.Equal(t, "frontend", opts.prefixDir) +} + +func TestParseNpmCLIArgs_WorkspaceBootstrap(t *testing.T) { + opts := parseNpmCLIArgs([]string{"install", "--workspace", "@scope/pkg", "-w"}) + assert.Equal(t, []string{"--workspace", "@scope/pkg", "-w"}, opts.bootstrapArgs) +} + +func TestEffectiveStartDir_PublishPathOverridesCwd(t *testing.T) { + got, err := effectiveStartDir("/repo", discoveryOptions{publishPath: "/repo/packages/foo"}) + assert.NoError(t, err) + assert.Equal(t, "/repo/packages/foo", got) +} + +func TestEffectiveStartDir_PrefixFromArgs(t *testing.T) { + got, err := effectiveStartDir("/repo", discoveryOptions{prefixDir: "sub"}) + assert.NoError(t, err) + assert.Equal(t, "/repo/sub", got) +} + +func TestBootstrapArgsFrom(t *testing.T) { + assert.Equal(t, []string{"-w"}, BootstrapArgsFrom([]string{"install", "-w"})) +} diff --git a/artifactory/healcomponents/npm/discover.go b/artifactory/healcomponents/npm/discover.go new file mode 100644 index 00000000..832c943b --- /dev/null +++ b/artifactory/healcomponents/npm/discover.go @@ -0,0 +1,68 @@ +package npm + +import ( + "os" + "path/filepath" + + "github.com/jfrog/jfrog-client-go/utils/errorutils" +) + +const ( + lockfileName = "package-lock.json" + shrinkwrapFileName = "npm-shrinkwrap.json" +) + +func discoverProjectRoot(workingDir string) (string, error) { + return discoverProjectRootWithOptions(workingDir, discoveryOptions{}) +} + +func discoverProjectRootWithOptions(workingDir string, opts discoveryOptions) (string, error) { + startDir, err := effectiveStartDir(workingDir, opts) + if err != nil { + return "", err + } + dir := startDir + var firstPackageJSON string + var topWorkspaceRoot string + for { + pkgPath := filepath.Join(dir, "package.json") + if _, statErr := os.Stat(pkgPath); statErr == nil { + if firstPackageJSON == "" { + firstPackageJSON = dir + } + if data, readErr := os.ReadFile(pkgPath); readErr == nil { + if pkg, parseErr := parsePackageJSON(data); parseErr == nil && pkg.hasWorkspaces() { + topWorkspaceRoot = dir + } + } + } + if _, statErr := os.Stat(filepath.Join(dir, shrinkwrapFileName)); statErr == nil { + return dir, nil + } + if _, statErr := os.Stat(filepath.Join(dir, lockfileName)); statErr == nil { + return dir, nil + } + parent := filepath.Dir(dir) + if parent == dir { + break + } + dir = parent + } + if topWorkspaceRoot != "" { + return topWorkspaceRoot, nil + } + if firstPackageJSON != "" { + return firstPackageJSON, nil + } + return "", errorutils.CheckErrorf("no %s or lockfile found from %s", "package.json", startDir) +} + +func lockfileNameInDir(dir string) (string, error) { + if _, err := os.Stat(filepath.Join(dir, shrinkwrapFileName)); err == nil { + return shrinkwrapFileName, nil + } + if _, err := os.Stat(filepath.Join(dir, lockfileName)); err == nil { + return lockfileName, nil + } + return "", errorutils.CheckErrorf("no %s or %s under %s", shrinkwrapFileName, lockfileName, dir) +} diff --git a/artifactory/healcomponents/npm/discover_test.go b/artifactory/healcomponents/npm/discover_test.go new file mode 100644 index 00000000..2ba27bb2 --- /dev/null +++ b/artifactory/healcomponents/npm/discover_test.go @@ -0,0 +1,92 @@ +package npm + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDiscoverProjectRoot_FromWorkspacePackage(t *testing.T) { + root := t.TempDir() + pkgDir := filepath.Join(root, "packages", "app") + require.NoError(t, os.MkdirAll(pkgDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(root, "package.json"), []byte(`{"workspaces":["packages/*"]}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(root, "package-lock.json"), []byte(`{}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(pkgDir, "package.json"), []byte(`{"name":"app"}`), 0644)) + + got, err := discoverProjectRoot(pkgDir) + require.NoError(t, err) + assert.Equal(t, root, got) +} + +func TestDiscoverProjectRoot_PrefixFlag(t *testing.T) { + root := t.TempDir() + sub := filepath.Join(root, "frontend") + require.NoError(t, os.MkdirAll(sub, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(sub, "package.json"), []byte(`{"name":"fe"}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(sub, "package-lock.json"), []byte(`{}`), 0644)) + + got, err := discoverProjectRootWithOptions(root, discoveryOptions{prefixDir: "frontend"}) + require.NoError(t, err) + assert.Equal(t, sub, got) +} + +func TestDiscoverProjectRoot_ShrinkwrapPrecedence(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte(`{"lock":1}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "npm-shrinkwrap.json"), []byte(`{"shrink":1}`), 0644)) + + name, err := lockfileNameInDir(dir) + require.NoError(t, err) + assert.Equal(t, shrinkwrapFileName, name) +} + +func TestDiscoverProjectRoot_IndependentPackageLock(t *testing.T) { + root := t.TempDir() + svc := filepath.Join(root, "svc-a") + require.NoError(t, os.MkdirAll(svc, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(root, "package.json"), []byte(`{"name":"root-no-ws"}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(svc, "package.json"), []byte(`{"name":"svc-a"}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(svc, "package-lock.json"), []byte(`{}`), 0644)) + + got, err := discoverProjectRoot(svc) + require.NoError(t, err) + assert.Equal(t, svc, got) +} + +func TestDiscoverProjectRoot_NoProjectFound(t *testing.T) { + dir := t.TempDir() + _, err := discoverProjectRoot(dir) + require.Error(t, err) + assert.Contains(t, err.Error(), "package.json") +} + +func TestDiscoverProjectRoot_IgnoresStrayPackageJSONWhenLockAbove(t *testing.T) { + root := t.TempDir() + svc := filepath.Join(root, "svc") + stray := filepath.Join(svc, "samples") + require.NoError(t, os.MkdirAll(stray, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(svc, "package.json"), []byte(`{"name":"svc"}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(svc, "package-lock.json"), []byte(`{}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(stray, "package.json"), []byte(`{"name":"sample"}`), 0644)) + + got, err := discoverProjectRoot(stray) + require.NoError(t, err) + assert.Equal(t, svc, got) +} + +func TestDiscoverProjectRoot_PublishPath(t *testing.T) { + root := t.TempDir() + pkg := filepath.Join(root, "packages", "lib") + require.NoError(t, os.MkdirAll(pkg, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(pkg, "package.json"), []byte(`{"name":"lib"}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(pkg, "package-lock.json"), []byte(`{}`), 0644)) + + got, err := discoverProjectRootWithOptions(root, discoveryOptions{publishPath: filepath.Join("packages", "lib")}) + require.NoError(t, err) + assert.Equal(t, pkg, got) +} diff --git a/artifactory/healcomponents/npm/npm.go b/artifactory/healcomponents/npm/npm.go new file mode 100644 index 00000000..f47eb56f --- /dev/null +++ b/artifactory/healcomponents/npm/npm.go @@ -0,0 +1,91 @@ +package npm + +import ( + "context" + "os" + "path/filepath" + + "github.com/jfrog/jfrog-client-go/utils/errorutils" + "github.com/jfrog/jfrog-client-go/utils/log" + + "github.com/jfrog/jfrog-cli-artifactory/artifactory/healcomponents" +) + +const toolName = "npm" + +type BuildTool struct { + opts discoveryOptions +} + +func NewBuildTool() BuildTool { + return BuildTool{} +} + +func NewBuildToolWithArgs(npmArgs []string) BuildTool { + parsed := parseNpmCLIArgs(npmArgs) + return BuildTool{opts: discoveryOptions{prefixDir: parsed.prefixDir}} +} + +func NewBuildToolForPublish(workingDir, publishPath string, npmArgs []string) BuildTool { + parsed := parseNpmCLIArgs(npmArgs) + return BuildTool{opts: discoveryOptions{ + prefixDir: parsed.prefixDir, + publishPath: publishPath, + }} +} + +func (BuildTool) ToolName() string { return toolName } + +func (BuildTool) RelevantCommands() []string { + return []string{"install", "ci", "publish"} +} + +func (t BuildTool) ProjectRoot(workingDir string) (string, error) { + return discoverProjectRootWithOptions(workingDir, t.opts) +} + +func (t BuildTool) EnsureLockfiles(ctx context.Context, projectRoot, command string, runner healcomponents.CommandRunner, bootstrapArgs ...string) ([]string, error) { + if _, err := os.Stat(filepath.Join(projectRoot, shrinkwrapFileName)); err == nil { + return nil, nil + } else if !os.IsNotExist(err) { + return nil, errorutils.CheckError(err) + } + lockPath := filepath.Join(projectRoot, lockfileName) + if _, err := os.Stat(lockPath); err == nil { + return nil, nil + } else if !os.IsNotExist(err) { + return nil, errorutils.CheckError(err) + } + switch command { + case "ci": + return nil, errorutils.CheckErrorf("component resolution requires %s or %s for npm ci (generate with npm install first)", lockfileName, shrinkwrapFileName) + case "install", "publish": + if runner == nil { + return nil, errorutils.CheckErrorf("npm runner required to bootstrap %s", lockfileName) + } + log.Info("Component resolution: generating ", lockfileName, " (lockfile was missing)") + args := append([]string{"install", "--package-lock-only"}, bootstrapArgs...) + if err := runner(ctx, projectRoot, args...); err != nil { + return nil, errorutils.CheckError(err) + } + return []string{lockfileName}, nil + default: + return nil, nil + } +} + +func (t BuildTool) DiscoverLockfiles(workingDir string) ([]healcomponents.Lockfile, error) { + root, err := discoverProjectRootWithOptions(workingDir, t.opts) + if err != nil { + return nil, err + } + name, err := lockfileNameInDir(root) + if err != nil { + return nil, err + } + data, err := os.ReadFile(filepath.Join(root, name)) + if err != nil { + return nil, errorutils.CheckError(err) + } + return []healcomponents.Lockfile{{Path: name, Content: data}}, nil +} diff --git a/artifactory/healcomponents/npm/npm_test.go b/artifactory/healcomponents/npm/npm_test.go new file mode 100644 index 00000000..04aeb13b --- /dev/null +++ b/artifactory/healcomponents/npm/npm_test.go @@ -0,0 +1,125 @@ +package npm + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNpmBuildTool_DiscoverLockfiles_SingleRootLock(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte(`{"lock":true}`), 0644)) + + tool := NewBuildTool() + files, err := tool.DiscoverLockfiles(dir) + require.NoError(t, err) + require.Len(t, files, 1) + assert.Equal(t, "package-lock.json", files[0].Path) + assert.JSONEq(t, `{"lock":true}`, string(files[0].Content)) +} + +func TestNpmBuildTool_DiscoverLockfiles_Shrinkwrap(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"a"}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "npm-shrinkwrap.json"), []byte(`{"s":1}`), 0644)) + + tool := NewBuildTool() + files, err := tool.DiscoverLockfiles(dir) + require.NoError(t, err) + require.Len(t, files, 1) + assert.Equal(t, "npm-shrinkwrap.json", files[0].Path) +} + +func TestNpmBuildTool_EnsureLockfiles_SkipsWhenShrinkwrapExists(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"a"}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "npm-shrinkwrap.json"), []byte(`{}`), 0644)) + + called := false + runner := func(context.Context, string, ...string) error { called = true; return nil } + + tool := NewBuildTool() + _, err := tool.EnsureLockfiles(context.Background(), dir, "install", runner) + require.NoError(t, err) + assert.False(t, called) +} + +func TestNpmBuildTool_EnsureLockfiles_CiRequiresExistingLock(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0644)) + + tool := NewBuildTool() + _, err := tool.EnsureLockfiles(context.Background(), dir, "ci", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "package-lock.json") +} + +func TestNpmBuildTool_EnsureLockfiles_InstallBootstrapsWhenMissing(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0644)) + + var ran []string + runner := func(_ context.Context, root string, args ...string) error { + ran = append(ran, root+":"+strings.Join(args, " ")) + require.NoError(t, os.WriteFile(filepath.Join(root, "package-lock.json"), []byte(`{"bootstrapped":true}`), 0644)) + return nil + } + + tool := NewBuildTool() + bootstrapped, err := tool.EnsureLockfiles(context.Background(), dir, "install", runner) + require.NoError(t, err) + assert.Equal(t, []string{"package-lock.json"}, bootstrapped) + assert.Contains(t, ran[0], "install --package-lock-only") +} + +func TestNpmBuildTool_EnsureLockfiles_SkipsBootstrapWhenLockExists(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte(`{}`), 0644)) + + called := false + runner := func(context.Context, string, ...string) error { + called = true + return nil + } + + tool := NewBuildTool() + _, err := tool.EnsureLockfiles(context.Background(), dir, "install", runner) + require.NoError(t, err) + assert.False(t, called) +} + +func TestNpmBuildTool_EnsureLockfiles_PassesWorkspaceFlags(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0644)) + + var ran []string + runner := func(_ context.Context, root string, args ...string) error { + ran = append(ran, strings.Join(args, " ")) + require.NoError(t, os.WriteFile(filepath.Join(root, "package-lock.json"), []byte(`{}`), 0644)) + return nil + } + + tool := NewBuildTool() + _, err := tool.EnsureLockfiles(context.Background(), dir, "publish", runner, "--workspaces") + require.NoError(t, err) + assert.Contains(t, ran[0], "--workspaces") +} + +func TestNewBuildToolWithArgs_Prefix(t *testing.T) { + tool := NewBuildToolWithArgs([]string{"install", "--prefix", "frontend"}) + root := t.TempDir() + sub := filepath.Join(root, "frontend") + require.NoError(t, os.MkdirAll(sub, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(sub, "package.json"), []byte(`{"name":"fe"}`), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(sub, "package-lock.json"), []byte(`{}`), 0644)) + + got, err := tool.ProjectRoot(root) + require.NoError(t, err) + assert.Equal(t, sub, got) +} diff --git a/artifactory/healcomponents/npm/package_json.go b/artifactory/healcomponents/npm/package_json.go new file mode 100644 index 00000000..6e66210a --- /dev/null +++ b/artifactory/healcomponents/npm/package_json.go @@ -0,0 +1,35 @@ +package npm + +import "encoding/json" + +type packageJSON struct { + Workspaces json.RawMessage `json:"workspaces"` +} + +func parsePackageJSON(data []byte) (packageJSON, error) { + var p packageJSON + if err := json.Unmarshal(data, &p); err != nil { + return packageJSON{}, err + } + return p, nil +} + +func (p packageJSON) hasWorkspaces() bool { + if len(p.Workspaces) == 0 || string(p.Workspaces) == "null" { + return false + } + if string(p.Workspaces) == "[]" { + return false + } + var arr []any + if json.Unmarshal(p.Workspaces, &arr) == nil { + return len(arr) > 0 + } + var obj struct { + Packages []any `json:"packages"` + } + if json.Unmarshal(p.Workspaces, &obj) == nil { + return len(obj.Packages) > 0 + } + return true +} diff --git a/artifactory/healcomponents/npm/package_json_test.go b/artifactory/healcomponents/npm/package_json_test.go new file mode 100644 index 00000000..92247ffc --- /dev/null +++ b/artifactory/healcomponents/npm/package_json_test.go @@ -0,0 +1,32 @@ +package npm + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParsePackageJSON_WorkspacesArray(t *testing.T) { + p, err := parsePackageJSON([]byte(`{"workspaces":["packages/*"]}`)) + require.NoError(t, err) + assert.True(t, p.hasWorkspaces()) +} + +func TestParsePackageJSON_WorkspacesObject(t *testing.T) { + p, err := parsePackageJSON([]byte(`{"workspaces":{"packages":["apps/*"],"nohoist":["**/react-native"]}}`)) + require.NoError(t, err) + assert.True(t, p.hasWorkspaces()) +} + +func TestParsePackageJSON_NoWorkspaces(t *testing.T) { + p, err := parsePackageJSON([]byte(`{"name":"app"}`)) + require.NoError(t, err) + assert.False(t, p.hasWorkspaces()) +} + +func TestParsePackageJSON_EmptyWorkspacesArray(t *testing.T) { + p, err := parsePackageJSON([]byte(`{"workspaces":[]}`)) + require.NoError(t, err) + assert.False(t, p.hasWorkspaces()) +} diff --git a/artifactory/healcomponents/service.go b/artifactory/healcomponents/service.go index 30eac495..27fcaacb 100644 --- a/artifactory/healcomponents/service.go +++ b/artifactory/healcomponents/service.go @@ -3,6 +3,7 @@ package healcomponents import ( "context" "os" + "path/filepath" "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/jfrog/jfrog-client-go/utils/log" @@ -25,6 +26,11 @@ type Lockfile struct { Content []byte } +type lockfileBackup struct { + path string + content []byte // nil means the file did not exist before apply +} + // ComponentResolutionClient resolves a single lockfile via Xray. type ComponentResolutionClient interface { HealComponents(req services.ComponentResolutionRequest) (*services.ComponentResolutionResponse, error) @@ -69,9 +75,9 @@ func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo st } toWrite = append(toWrite, Lockfile{Path: lf.Path, Content: []byte(resp.Lockfile)}) totalChanges += len(resp.Changes) - log.Debug("Healed ", lf.Path, " with ", len(resp.Changes), " package change(s)") + log.Debug("Healed", lf.Path, "with", len(resp.Changes), "package change(s)") for _, ch := range resp.Changes { - log.Debug(" ", ch.Package, ": ", ch.BeforeIntegrity, " → ", ch.AfterIntegrity) + log.Debug(" ", ch.Package, ":", ch.BeforeIntegrity, "→", ch.AfterIntegrity) } } if len(toWrite) == 0 { @@ -82,7 +88,7 @@ func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo st if err != nil { return noopRestore, false, err } - log.Info("Xray component resolution healed ", totalChanges, " package change(s) across ", len(toWrite), " lockfile(s)") + log.Info("Xray component resolution healed", totalChanges, "package change(s) across", len(toWrite), "lockfile(s)") return restore, true, nil } @@ -93,3 +99,53 @@ func getLockfilePaths(lockfiles []Lockfile) []string { } return paths } + +// ApplyLockfiles backs up existing lockfiles under projectRoot, writes updates, returns restore func. +// Paths listed in treatAsAbsent are restored by deletion even if they exist on disk (bootstrapped locks). +func ApplyLockfiles(projectRoot string, lockfiles []Lockfile, treatAsAbsent []string) (restore func() error, err error) { + if len(lockfiles) == 0 { + return func() error { return nil }, nil + } + + absent := make(map[string]bool, len(treatAsAbsent)) + for _, path := range treatAsAbsent { + absent[path] = true + } + + var backups []lockfileBackup + for _, lf := range lockfiles { + fullPath := filepath.Join(projectRoot, lf.Path) + backup := lockfileBackup{path: fullPath} + if !absent[lf.Path] { + data, readErr := os.ReadFile(fullPath) + if readErr == nil { + backup.content = data + } else if !os.IsNotExist(readErr) { + return nil, errorutils.CheckError(readErr) + } + } + backups = append(backups, backup) + + if err = os.MkdirAll(filepath.Dir(fullPath), 0755); err != nil { + return nil, errorutils.CheckError(err) + } + if err = os.WriteFile(fullPath, lf.Content, 0644); err != nil { + return nil, errorutils.CheckError(err) + } + } + + return func() error { + for _, backup := range backups { + if backup.content == nil { + if err = os.Remove(backup.path); err != nil && !os.IsNotExist(err) { + return errorutils.CheckError(err) + } + continue + } + if err = os.WriteFile(backup.path, backup.content, 0644); err != nil { + return errorutils.CheckError(err) + } + } + return nil + }, nil +} diff --git a/artifactory/healcomponents/service_test.go b/artifactory/healcomponents/service_test.go index fb4a320e..3f2a7f10 100644 --- a/artifactory/healcomponents/service_test.go +++ b/artifactory/healcomponents/service_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + testsutils "github.com/jfrog/jfrog-client-go/utils/tests" "github.com/jfrog/jfrog-client-go/xray/services" ) @@ -181,3 +182,24 @@ func TestRunIfEnabled_PropagatesEnsureLockfilesError(t *testing.T) { assert.Contains(t, err.Error(), "package-lock.json") assert.Equal(t, 0, client.callCount) } + +func TestApplyLockfiles_WritesMultipleFiles(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte("a"), 0644)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "app"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "app/gradle.lockfile"), []byte("b"), 0644)) + + restore, err := ApplyLockfiles(dir, []Lockfile{ + {Path: "package-lock.json", Content: []byte("a-healed")}, + {Path: "app/gradle.lockfile", Content: []byte("b-healed")}, + }, nil) + require.NoError(t, err) + defer testsutils.RemoveAllAndAssert(t, dir) + + a, _ := os.ReadFile(filepath.Join(dir, "package-lock.json")) + b, _ := os.ReadFile(filepath.Join(dir, "app/gradle.lockfile")) + assert.Equal(t, "a-healed", string(a)) + assert.Equal(t, "b-healed", string(b)) + + require.NoError(t, restore()) +} From bd6d47ef6513c7d8e0a9d085abf53641d456e9a9 Mon Sep 17 00:00:00 2001 From: attiasas Date: Thu, 18 Jun 2026 10:56:36 +0300 Subject: [PATCH 7/9] add min version tests --- artifactory/healcomponents/service.go | 13 ++++ artifactory/healcomponents/service_test.go | 76 +++++++++++++++++++++- go.mod | 4 +- go.sum | 8 +-- 4 files changed, 92 insertions(+), 9 deletions(-) diff --git a/artifactory/healcomponents/service.go b/artifactory/healcomponents/service.go index 27fcaacb..e6a3d209 100644 --- a/artifactory/healcomponents/service.go +++ b/artifactory/healcomponents/service.go @@ -8,12 +8,15 @@ import ( "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/jfrog/jfrog-client-go/utils/log" "github.com/jfrog/jfrog-client-go/xray/services" + clientutils "github.com/jfrog/jfrog-client-go/utils" ) // HealComponentsDisabledEnvVar disables Xray heal components when set to "true". // Feature is enabled by default; users run normal jf commands with no extra setup. const HealComponentsDisabledEnvVar = "JFROG_CLI_HEAL_COMPONENTS_DISABLED" +const HealComponentsMinVersion = "3.133.0" + var noopRestore = func() error { return nil } func IsComponentResolutionDisabled() bool { @@ -33,6 +36,7 @@ type lockfileBackup struct { // ComponentResolutionClient resolves a single lockfile via Xray. type ComponentResolutionClient interface { + GetVersion() (string, error) HealComponents(req services.ComponentResolutionRequest) (*services.ComponentResolutionResponse, error) } @@ -43,6 +47,15 @@ func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo st log.Debug("Xray heal components disabled or not relevant command: ", command) return noopRestore, false, nil } + version, err := client.GetVersion() + if err != nil { + return noopRestore, false, err + } + log.Debug("Xray version: ", version) + if err = clientutils.ValidateMinimumVersion(clientutils.Xray, version, HealComponentsMinVersion); err != nil { + log.Warn("Xray heal components are not supported on the current Xray version. " + err.Error()) + return noopRestore, false, nil + } log.Debug("Running Xray heal components at '"+repo+"' RT repository for tool:", tool.ToolName()) projectRoot, err := tool.ProjectRoot(workingDir) if err != nil { diff --git a/artifactory/healcomponents/service_test.go b/artifactory/healcomponents/service_test.go index 3f2a7f10..d9425f91 100644 --- a/artifactory/healcomponents/service_test.go +++ b/artifactory/healcomponents/service_test.go @@ -15,9 +15,21 @@ import ( ) type mockClient struct { - callCount int - lastReq services.ComponentResolutionRequest - resp services.ComponentResolutionResponse + callCount int + lastReq services.ComponentResolutionRequest + resp services.ComponentResolutionResponse + version string + versionErr error +} + +func (m *mockClient) GetVersion() (string, error) { + if m.versionErr != nil { + return "", m.versionErr + } + if m.version != "" { + return m.version, nil + } + return HealComponentsMinVersion, nil } func (m *mockClient) HealComponents(req services.ComponentResolutionRequest) (*services.ComponentResolutionResponse, error) { @@ -183,6 +195,64 @@ func TestRunIfEnabled_PropagatesEnsureLockfilesError(t *testing.T) { assert.Equal(t, 0, client.callCount) } +func TestRunIfEnabled_PropagatesGetVersionError(t *testing.T) { + client := &mockClient{versionErr: errors.New("xray unavailable")} + tool := mockTool{root: t.TempDir(), lockfiles: []Lockfile{{Path: "package-lock.json", Content: []byte("orig")}}} + _, healed, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", t.TempDir(), nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "xray unavailable") + assert.False(t, healed) + assert.Equal(t, 0, client.callCount) +} + +func TestRunIfEnabled_SkipsWhenXrayVersionTooLow(t *testing.T) { + dir := t.TempDir() + lockPath := filepath.Join(dir, "package-lock.json") + require.NoError(t, os.WriteFile(lockPath, []byte("orig"), 0644)) + + client := &mockClient{ + version: "3.100.0", + resp: services.ComponentResolutionResponse{ + Lockfile: "healed", + Changes: []services.Change{{Package: "lodash", BeforeIntegrity: "a", AfterIntegrity: "b"}}, + }, + } + tool := mockTool{root: dir, lockfiles: []Lockfile{{Path: "package-lock.json", Content: []byte("orig")}}} + + _, healed, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", dir, nil) + require.NoError(t, err) + assert.False(t, healed) + assert.Equal(t, 0, client.callCount) + + data, err := os.ReadFile(lockPath) + require.NoError(t, err) + assert.Equal(t, "orig", string(data)) +} + +func TestRunIfEnabled_AllowsXrayDevVersion(t *testing.T) { + dir := t.TempDir() + lockPath := filepath.Join(dir, "package-lock.json") + require.NoError(t, os.WriteFile(lockPath, []byte("orig"), 0644)) + + client := &mockClient{ + version: "3.x-dev", + resp: services.ComponentResolutionResponse{ + Lockfile: "healed", + Changes: []services.Change{{Package: "lodash", BeforeIntegrity: "a", AfterIntegrity: "b"}}, + }, + } + tool := mockTool{root: dir, lockfiles: []Lockfile{{Path: "package-lock.json", Content: []byte("orig")}}} + + _, healed, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", dir, nil) + require.NoError(t, err) + assert.True(t, healed) + assert.Equal(t, 1, client.callCount) + + data, err := os.ReadFile(lockPath) + require.NoError(t, err) + assert.Equal(t, "healed", string(data)) +} + func TestApplyLockfiles_WritesMultipleFiles(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte("a"), 0644)) diff --git a/go.mod b/go.mod index 9431b91c..3d9167f3 100644 --- a/go.mod +++ b/go.mod @@ -203,10 +203,10 @@ require ( // replace github.com/jfrog/jfrog-cli-core/v2 => /Users/assafa/code/jfrog/jfrog-cli-core // attiasas:xray_component_resolution -replace github.com/jfrog/jfrog-client-go => github.com/attiasas/jfrog-client-go v0.0.0-20260617184739-f8431de360c6 +replace github.com/jfrog/jfrog-client-go => github.com/attiasas/jfrog-client-go v0.0.0-20260618074648-a1bc0ef2898a // attiasas:expend_xray_manager -replace github.com/jfrog/jfrog-cli-core/v2 => github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617184845-0cfaec9c03c7 +replace github.com/jfrog/jfrog-cli-core/v2 => github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260618075235-99c87dda5bb2 // replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260604085947-7c110b77b4b4 diff --git a/go.sum b/go.sum index c238a448..1e926dcd 100644 --- a/go.sum +++ b/go.sum @@ -59,10 +59,10 @@ github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPd github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= -github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617184845-0cfaec9c03c7 h1:T7V2yJJFyKh+cf1thzTuAZThD/nXDH3GDHNCFyOzM/s= -github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260617184845-0cfaec9c03c7/go.mod h1:Tf6NEshuw3NShJabLP1/FHuATGBhWKW/6WjETi0LhKc= -github.com/attiasas/jfrog-client-go v0.0.0-20260617184739-f8431de360c6 h1:OwS+TQYZvx+PiZgT043h7nagc5yYG410HG8JZGPP9FU= -github.com/attiasas/jfrog-client-go v0.0.0-20260617184739-f8431de360c6/go.mod h1:FHpjN1nTDoj96xd6obe27EOgGErqzU0rQgC96L3Ch9E= +github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260618075235-99c87dda5bb2 h1:kc/SEAhQqaE2daomMU0Vt90aNrGryzCvSsOnqrjozTo= +github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260618075235-99c87dda5bb2/go.mod h1:LPbM+pIqZ43Pg2rTGweU1V+aIXKAdbkm2Srw6Jb0aiY= +github.com/attiasas/jfrog-client-go v0.0.0-20260618074648-a1bc0ef2898a h1:6TIh27YOWeMz1zd7/5jOi3qZ/oqVQxGsSjdKZju4rFk= +github.com/attiasas/jfrog-client-go v0.0.0-20260618074648-a1bc0ef2898a/go.mod h1:FHpjN1nTDoj96xd6obe27EOgGErqzU0rQgC96L3Ch9E= github.com/aws/aws-sdk-go v1.55.8 h1:JRmEUbU52aJQZ2AjX4q4Wu7t4uZjOu71uyNmaWlUkJQ= github.com/aws/aws-sdk-go v1.55.8/go.mod h1:ZkViS9AqA6otK+JBBNH2++sx1sgxrPKcSzPPvQkUtXk= github.com/aws/aws-sdk-go-v2 v1.41.1 h1:ABlyEARCDLN034NhxlRUSZr4l71mh+T5KAeGh6cerhU= From 8908efab688af4fe08491f77c62cfd90b96430a0 Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 23 Jun 2026 14:09:13 +0300 Subject: [PATCH 8/9] update deps, skip if disabled --- artifactory/commands/npm/publish.go | 2 +- artifactory/commands/npm/xrayheal.go | 21 +++++++------ artifactory/healcomponents/service.go | 34 ++++++++++++++++++---- artifactory/healcomponents/service_test.go | 23 +++++++++++++-- go.mod | 4 +-- go.sum | 8 ++--- 6 files changed, 66 insertions(+), 26 deletions(-) diff --git a/artifactory/commands/npm/publish.go b/artifactory/commands/npm/publish.go index b54e6acd..3b3176c8 100644 --- a/artifactory/commands/npm/publish.go +++ b/artifactory/commands/npm/publish.go @@ -79,7 +79,7 @@ func (npc *NpmPublishCommand) ServerDetails() (*config.ServerDetails, error) { func (npc *NpmPublishCommand) SetConfigFilePath(configFilePath string) *NpmPublishCommand { npc.configFilePath = configFilePath - npc.SetConfigFilePath(configFilePath) + npc.CommonArgs.SetConfigFilePath(configFilePath) return npc } diff --git a/artifactory/commands/npm/xrayheal.go b/artifactory/commands/npm/xrayheal.go index d1448a18..643f40d7 100644 --- a/artifactory/commands/npm/xrayheal.go +++ b/artifactory/commands/npm/xrayheal.go @@ -3,7 +3,6 @@ package npm import ( "context" "fmt" - "github.com/jfrog/jfrog-client-go/utils/log" "strings" gofrogcmd "github.com/jfrog/gofrog/io" @@ -27,23 +26,23 @@ func (ca *CommonArgs) runXrayComponentHealingForPublish(ctx context.Context, com } func (ca *CommonArgs) runXrayComponentHealingWithTool(ctx context.Context, command, workingDir string, tool cnpm.BuildTool, bootstrapArgs ...string) (restore func() error, healed bool, err error) { - noop := func() error { return nil } if healcomponents.IsComponentResolutionDisabled() { - return noop, false, nil + return healcomponents.SkipHealing("Xray component healing disabled", nil) } - resolverRepo, err := ca.resolverRepoForResolution(command) - if err != nil || resolverRepo == "" { - log.Debug("Xray component healing skipped: could not determine resolver repo: " + err.Error()) - return noop, false, nil + resolverRepo, resolverErr := ca.resolverRepoForResolution(command) + if resolverErr != nil { + return healcomponents.SkipHealing("Xray component healing skipped: could not determine resolver repo: ", resolverErr) + } + if resolverRepo == "" { + return healcomponents.SkipHealing("Xray component healing skipped: resolver repo is empty", nil) } var projectKey string if ca.buildConfiguration != nil { projectKey = ca.buildConfiguration.GetProject() } - xrayManager, err := xray.CreateXrayServiceManager(ca.serverDetails, xray.WithScopedProjectKey(projectKey)) - if err != nil { - log.Debug("Xray component healing skipped: could not create Xray service manager: " + err.Error()) - return noop, false, nil + xrayManager, xrayErr := xray.CreateXrayServiceManager(ca.serverDetails, xray.WithScopedProjectKey(projectKey)) + if xrayErr != nil { + return healcomponents.SkipHealing("Xray component healing skipped: could not create Xray service manager: ", xrayErr) } return healcomponents.RunIfEnabled(ctx, xrayManager, resolverRepo, tool, command, workingDir, ca.npmBootstrapRunner(), bootstrapArgs...) } diff --git a/artifactory/healcomponents/service.go b/artifactory/healcomponents/service.go index e6a3d209..e29f876c 100644 --- a/artifactory/healcomponents/service.go +++ b/artifactory/healcomponents/service.go @@ -5,10 +5,10 @@ import ( "os" "path/filepath" + clientutils "github.com/jfrog/jfrog-client-go/utils" "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/jfrog/jfrog-client-go/utils/log" "github.com/jfrog/jfrog-client-go/xray/services" - clientutils "github.com/jfrog/jfrog-client-go/utils" ) // HealComponentsDisabledEnvVar disables Xray heal components when set to "true". @@ -19,6 +19,25 @@ const HealComponentsMinVersion = "3.133.0" var noopRestore = func() error { return nil } +// SkipHealing logs and returns without error. Component healing is best-effort and must not fail the caller's build. +func SkipHealing(message string, cause error) (func() error, bool, error) { + if cause != nil { + log.Debug(message + cause.Error()) + } else { + log.Debug(message) + } + return noopRestore, false, nil +} + +func skipHealingWarn(message string, cause error) (func() error, bool, error) { + if cause != nil { + log.Warn(message + cause.Error()) + } else { + log.Warn(message) + } + return noopRestore, false, nil +} + func IsComponentResolutionDisabled() bool { return os.Getenv(HealComponentsDisabledEnvVar) == "true" } @@ -37,7 +56,7 @@ type lockfileBackup struct { // ComponentResolutionClient resolves a single lockfile via Xray. type ComponentResolutionClient interface { GetVersion() (string, error) - HealComponents(req services.ComponentResolutionRequest) (*services.ComponentResolutionResponse, error) + HealComponents(req services.ComponentResolutionRequest) (*services.ComponentResolutionResponse, bool, error) } // RunIfEnabled ensures lockfiles exist, discovers them, calls Xray once per file, writes healed lockfiles when changes returned. @@ -52,9 +71,8 @@ func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo st return noopRestore, false, err } log.Debug("Xray version: ", version) - if err = clientutils.ValidateMinimumVersion(clientutils.Xray, version, HealComponentsMinVersion); err != nil { - log.Warn("Xray heal components are not supported on the current Xray version. " + err.Error()) - return noopRestore, false, nil + if versionErr := clientutils.ValidateMinimumVersion(clientutils.Xray, version, HealComponentsMinVersion); versionErr != nil { + return skipHealingWarn("Xray heal components are not supported on the current Xray version. ", versionErr) } log.Debug("Running Xray heal components at '"+repo+"' RT repository for tool:", tool.ToolName()) projectRoot, err := tool.ProjectRoot(workingDir) @@ -74,7 +92,7 @@ func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo st var toWrite []Lockfile var totalChanges int for _, lf := range lockfiles { - resp, err := client.HealComponents(services.ComponentResolutionRequest{ + resp, disabled, err := client.HealComponents(services.ComponentResolutionRequest{ BuildTool: tool.ToolName(), Repo: repo, Lockfile: string(lf.Content), @@ -82,6 +100,10 @@ func RunIfEnabled(ctx context.Context, client ComponentResolutionClient, repo st if err != nil { return noopRestore, false, errorutils.CheckError(err) } + if disabled { + log.Debug("Xray component healing skipped: self-heal is disabled on the server") + return noopRestore, false, nil + } if len(resp.Changes) == 0 { log.Debug("No changes for ", lf.Path) continue diff --git a/artifactory/healcomponents/service_test.go b/artifactory/healcomponents/service_test.go index d9425f91..e16373a7 100644 --- a/artifactory/healcomponents/service_test.go +++ b/artifactory/healcomponents/service_test.go @@ -18,6 +18,7 @@ type mockClient struct { callCount int lastReq services.ComponentResolutionRequest resp services.ComponentResolutionResponse + disabled bool version string versionErr error } @@ -32,10 +33,10 @@ func (m *mockClient) GetVersion() (string, error) { return HealComponentsMinVersion, nil } -func (m *mockClient) HealComponents(req services.ComponentResolutionRequest) (*services.ComponentResolutionResponse, error) { +func (m *mockClient) HealComponents(req services.ComponentResolutionRequest) (*services.ComponentResolutionResponse, bool, error) { m.callCount++ m.lastReq = req - return &m.resp, nil + return &m.resp, m.disabled, nil } type mockTool struct { @@ -133,6 +134,24 @@ func TestRunIfEnabled_WritesHealedNpmLockAsString(t *testing.T) { assert.Equal(t, orig, client.lastReq.Lockfile) } +func TestRunIfEnabled_SkipsWhenServiceDisabled(t *testing.T) { + dir := t.TempDir() + lockPath := filepath.Join(dir, "package-lock.json") + require.NoError(t, os.WriteFile(lockPath, []byte("orig"), 0644)) + + client := &mockClient{disabled: true} + tool := mockTool{root: dir, lockfiles: []Lockfile{{Path: "package-lock.json", Content: []byte("orig")}}} + + _, healed, err := RunIfEnabled(context.Background(), client, "npm-virtual", tool, "install", dir, nil) + require.NoError(t, err) + assert.False(t, healed) + assert.Equal(t, 1, client.callCount) + + data, err := os.ReadFile(lockPath) + require.NoError(t, err) + assert.Equal(t, "orig", string(data)) +} + func TestRunIfEnabled_SkipsWhenDisabled(t *testing.T) { t.Setenv(HealComponentsDisabledEnvVar, "true") client := &mockClient{} diff --git a/go.mod b/go.mod index 3d9167f3..a93c4c3e 100644 --- a/go.mod +++ b/go.mod @@ -203,10 +203,10 @@ require ( // replace github.com/jfrog/jfrog-cli-core/v2 => /Users/assafa/code/jfrog/jfrog-cli-core // attiasas:xray_component_resolution -replace github.com/jfrog/jfrog-client-go => github.com/attiasas/jfrog-client-go v0.0.0-20260618074648-a1bc0ef2898a +replace github.com/jfrog/jfrog-client-go => github.com/attiasas/jfrog-client-go v0.0.0-20260623101014-8f8f09cbd450 // attiasas:expend_xray_manager -replace github.com/jfrog/jfrog-cli-core/v2 => github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260618075235-99c87dda5bb2 +replace github.com/jfrog/jfrog-cli-core/v2 => github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260623101547-db7e26e572b3 // replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260604085947-7c110b77b4b4 diff --git a/go.sum b/go.sum index 1e926dcd..f9feaadc 100644 --- a/go.sum +++ b/go.sum @@ -59,10 +59,10 @@ github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPd github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= -github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260618075235-99c87dda5bb2 h1:kc/SEAhQqaE2daomMU0Vt90aNrGryzCvSsOnqrjozTo= -github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260618075235-99c87dda5bb2/go.mod h1:LPbM+pIqZ43Pg2rTGweU1V+aIXKAdbkm2Srw6Jb0aiY= -github.com/attiasas/jfrog-client-go v0.0.0-20260618074648-a1bc0ef2898a h1:6TIh27YOWeMz1zd7/5jOi3qZ/oqVQxGsSjdKZju4rFk= -github.com/attiasas/jfrog-client-go v0.0.0-20260618074648-a1bc0ef2898a/go.mod h1:FHpjN1nTDoj96xd6obe27EOgGErqzU0rQgC96L3Ch9E= +github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260623101547-db7e26e572b3 h1:3zg6St+tq0Mt6NsmrY9PlTvD5y5YgdRfxL2qf5pI7/c= +github.com/attiasas/jfrog-cli-core/v2 v2.0.0-20260623101547-db7e26e572b3/go.mod h1:X4RdJAWdHMXZLBcvBeC9dYR12JKysbVNIRngtk+N7M8= +github.com/attiasas/jfrog-client-go v0.0.0-20260623101014-8f8f09cbd450 h1:Kk04jzpBs31RdhaHVZ+WBY8WSQgCKZCDcymc2M/5IoI= +github.com/attiasas/jfrog-client-go v0.0.0-20260623101014-8f8f09cbd450/go.mod h1:FHpjN1nTDoj96xd6obe27EOgGErqzU0rQgC96L3Ch9E= github.com/aws/aws-sdk-go v1.55.8 h1:JRmEUbU52aJQZ2AjX4q4Wu7t4uZjOu71uyNmaWlUkJQ= github.com/aws/aws-sdk-go v1.55.8/go.mod h1:ZkViS9AqA6otK+JBBNH2++sx1sgxrPKcSzPPvQkUtXk= github.com/aws/aws-sdk-go-v2 v1.41.1 h1:ABlyEARCDLN034NhxlRUSZr4l71mh+T5KAeGh6cerhU= From 06a8a907fa5d74b4254cccea33e7b099d365d1af Mon Sep 17 00:00:00 2001 From: attiasas Date: Wed, 24 Jun 2026 09:16:16 +0300 Subject: [PATCH 9/9] fix tests --- artifactory/healcomponents/npm/args_parse.go | 28 ++++++++++++------- .../healcomponents/npm/args_parse_test.go | 24 ++++++++++++++-- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/artifactory/healcomponents/npm/args_parse.go b/artifactory/healcomponents/npm/args_parse.go index 751b8bbd..9237fa0c 100644 --- a/artifactory/healcomponents/npm/args_parse.go +++ b/artifactory/healcomponents/npm/args_parse.go @@ -59,18 +59,26 @@ func effectiveStartDir(workingDir string, opts discoveryOptions) (string, error) return "", errorutils.CheckError(err) } if opts.publishPath != "" { - p := opts.publishPath - if !filepath.IsAbs(p) { - p = filepath.Join(abs, p) - } - return filepath.Clean(p), nil + return resolveDiscoveryPath(abs, opts.publishPath) } if opts.prefixDir != "" { - p := opts.prefixDir - if !filepath.IsAbs(p) { - p = filepath.Join(abs, p) - } - return filepath.Clean(p), nil + return resolveDiscoveryPath(abs, opts.prefixDir) } return abs, nil } + +// resolveDiscoveryPath joins base and p unless p is already absolute. +// On Windows, Unix-style paths (e.g. /repo/pkg) are not filepath.IsAbs but must not be joined with base. +func resolveDiscoveryPath(base, p string) (string, error) { + if filepath.IsAbs(p) { + return filepath.Clean(p), nil + } + if strings.HasPrefix(filepath.ToSlash(p), "/") { + abs, err := filepath.Abs(p) + if err != nil { + return "", errorutils.CheckError(err) + } + return filepath.Clean(abs), nil + } + return filepath.Clean(filepath.Join(base, p)), nil +} diff --git a/artifactory/healcomponents/npm/args_parse_test.go b/artifactory/healcomponents/npm/args_parse_test.go index ad76a4c3..e63019a6 100644 --- a/artifactory/healcomponents/npm/args_parse_test.go +++ b/artifactory/healcomponents/npm/args_parse_test.go @@ -1,6 +1,7 @@ package npm import ( + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -27,15 +28,34 @@ func TestParseNpmCLIArgs_WorkspaceBootstrap(t *testing.T) { } func TestEffectiveStartDir_PublishPathOverridesCwd(t *testing.T) { + root := t.TempDir() + publishPath := filepath.Join(root, "packages", "foo") + got, err := effectiveStartDir(root, discoveryOptions{publishPath: publishPath}) + assert.NoError(t, err) + assert.Equal(t, publishPath, got) +} + +func TestEffectiveStartDir_PublishPathUnixAbsolute(t *testing.T) { got, err := effectiveStartDir("/repo", discoveryOptions{publishPath: "/repo/packages/foo"}) assert.NoError(t, err) - assert.Equal(t, "/repo/packages/foo", got) + want, err := filepath.Abs("/repo/packages/foo") + assert.NoError(t, err) + assert.Equal(t, want, got) } func TestEffectiveStartDir_PrefixFromArgs(t *testing.T) { + root := t.TempDir() + got, err := effectiveStartDir(root, discoveryOptions{prefixDir: "sub"}) + assert.NoError(t, err) + assert.Equal(t, filepath.Join(root, "sub"), got) +} + +func TestEffectiveStartDir_PrefixFromArgsUnixRoot(t *testing.T) { got, err := effectiveStartDir("/repo", discoveryOptions{prefixDir: "sub"}) assert.NoError(t, err) - assert.Equal(t, "/repo/sub", got) + root, err := filepath.Abs("/repo") + assert.NoError(t, err) + assert.Equal(t, filepath.Join(root, "sub"), got) } func TestBootstrapArgsFrom(t *testing.T) {