From c1c6d034fccfa5294ab6ba84c94ea9bb0ea94d24 Mon Sep 17 00:00:00 2001 From: Gauri Yadav Date: Thu, 13 Aug 2026 15:34:27 +0530 Subject: [PATCH] XRAY-157755 - Fix insecure tls flag traversal for maven projects --- sca/bom/buildinfo/buildinfobom.go | 27 +-- sca/bom/buildinfo/buildinfobom_test.go | 10 ++ .../technologies/java/deptreemanager.go | 1 + sca/bom/buildinfo/technologies/java/mvn.go | 52 +++++- .../buildinfo/technologies/java/mvn_test.go | 168 ++++++++++++++++++ 5 files changed, 245 insertions(+), 13 deletions(-) diff --git a/sca/bom/buildinfo/buildinfobom.go b/sca/bom/buildinfo/buildinfobom.go index bc0c7be4f..bc77b6858 100644 --- a/sca/bom/buildinfo/buildinfobom.go +++ b/sca/bom/buildinfo/buildinfobom.go @@ -261,17 +261,7 @@ func GetTechDependencyTree(params technologies.BuildInfoBomGeneratorParams, arti switch tech { case techutils.Maven, techutils.Gradle: - depTreeResult.FullDepTrees, uniqDepsNodes, err = java.BuildDependencyTree(java.DepTreeParams{ - Server: artifactoryServerDetails, - DepsRepo: params.DependenciesRepository, - IsMavenDepTreeInstalled: params.IsMavenDepTreeInstalled, - UseWrapper: params.UseWrapper, - IsCurationCmd: params.IsCurationCmd, - MvnIncludePluginDeps: params.MvnIncludePluginDeps, - CurationCacheFolder: curationCacheFolder, - UseIncludedBuilds: params.UseIncludedBuilds, - GradleExcludeTestDependencies: params.GradleExcludeTestDependencies, - }, tech) + depTreeResult.FullDepTrees, uniqDepsNodes, err = java.BuildDependencyTree(buildJavaDepTreeParams(params, artifactoryServerDetails, curationCacheFolder), tech) case techutils.Npm: depTreeResult.FullDepTrees, uniqueDepsIds, err = npm.BuildDependencyTree(params) case techutils.Pnpm: @@ -322,6 +312,21 @@ func GetTechDependencyTree(params technologies.BuildInfoBomGeneratorParams, arti return } +func buildJavaDepTreeParams(params technologies.BuildInfoBomGeneratorParams, artifactoryServerDetails *config.ServerDetails, curationCacheFolder string) java.DepTreeParams { + return java.DepTreeParams{ + Server: artifactoryServerDetails, + DepsRepo: params.DependenciesRepository, + InsecureTls: params.InsecureTls, + IsMavenDepTreeInstalled: params.IsMavenDepTreeInstalled, + UseWrapper: params.UseWrapper, + IsCurationCmd: params.IsCurationCmd, + MvnIncludePluginDeps: params.MvnIncludePluginDeps, + CurationCacheFolder: curationCacheFolder, + UseIncludedBuilds: params.UseIncludedBuilds, + GradleExcludeTestDependencies: params.GradleExcludeTestDependencies, + } +} + func getUniqueDependencyCount(uniqueDepsIds []string, uniqDepsNodes map[string]*xray.DepTreeNode) int { if len(uniqDepsNodes) > 0 { return len(uniqDepsNodes) diff --git a/sca/bom/buildinfo/buildinfobom_test.go b/sca/bom/buildinfo/buildinfobom_test.go index 5c64592af..180e63361 100644 --- a/sca/bom/buildinfo/buildinfobom_test.go +++ b/sca/bom/buildinfo/buildinfobom_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/CycloneDX/cyclonedx-go" + "github.com/jfrog/jfrog-cli-security/sca/bom/buildinfo/technologies" "github.com/jfrog/jfrog-cli-security/utils/results" xrayUtils "github.com/jfrog/jfrog-client-go/xray/services/utils" @@ -253,3 +254,12 @@ func TestGetDiffDependencyTree(t *testing.T) { }) } } + +func TestBuildJavaDepTreeParamsPreservesInsecureTls(t *testing.T) { + t.Parallel() + params := technologies.BuildInfoBomGeneratorParams{InsecureTls: true, DependenciesRepository: "test-repo"} + result := buildJavaDepTreeParams(params, nil, "cache-folder") + assert.True(t, result.InsecureTls) + assert.Equal(t, "test-repo", result.DepsRepo) + assert.Equal(t, "cache-folder", result.CurationCacheFolder) +} diff --git a/sca/bom/buildinfo/technologies/java/deptreemanager.go b/sca/bom/buildinfo/technologies/java/deptreemanager.go index 17fff2a83..a736ec880 100644 --- a/sca/bom/buildinfo/technologies/java/deptreemanager.go +++ b/sca/bom/buildinfo/technologies/java/deptreemanager.go @@ -30,6 +30,7 @@ type DepTreeParams struct { UseWrapper bool Server *config.ServerDetails DepsRepo string + InsecureTls bool IsMavenDepTreeInstalled bool IsCurationCmd bool MvnIncludePluginDeps bool diff --git a/sca/bom/buildinfo/technologies/java/mvn.go b/sca/bom/buildinfo/technologies/java/mvn.go index e55e604d4..85ad1e4fc 100644 --- a/sca/bom/buildinfo/technologies/java/mvn.go +++ b/sca/bom/buildinfo/technologies/java/mvn.go @@ -12,12 +12,14 @@ import ( "path/filepath" "strings" "text/template" + "unicode/utf8" "github.com/beevik/etree" "github.com/jfrog/jfrog-cli-security/sca/bom/buildinfo/technologies" "github.com/jfrog/jfrog-cli-security/utils/techutils" "github.com/jfrog/jfrog-cli-security/utils/xray" + "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-cli-core/v2/utils/ioutils" "github.com/jfrog/jfrog-client-go/utils/errorutils" @@ -66,6 +68,7 @@ type MavenDepTreeManager struct { settingsXmlPath string // userSettingsXmlPath overrides the ~/.m2/settings.xml seed path (test-only). userSettingsXmlPath string + insecureTls bool } func NewMavenDepTreeManager(params *DepTreeParams, cmdName MavenDepTreeCmd) *MavenDepTreeManager { @@ -77,6 +80,7 @@ func NewMavenDepTreeManager(params *DepTreeParams, cmdName MavenDepTreeCmd) *Mav isCurationCmd: params.IsCurationCmd, mvnIncludePluginDeps: params.MvnIncludePluginDeps, curationCacheFolder: params.CurationCacheFolder, + insecureTls: params.InsecureTls, } } @@ -220,24 +224,68 @@ func (mdt *MavenDepTreeManager) RunMvnCmd(goals []string) (cmdOutput []byte, err if mdt.settingsXmlPath != "" { goals = append(goals, "-s", mdt.settingsXmlPath) } + if mdt.insecureTls { + // aether.* covers Maven 3.9+'s native resolver transport; wagon.* covers the legacy one. + goals = append(goals, + "-Dmaven.wagon.http.ssl.insecure=true", + "-Dmaven.wagon.http.ssl.allowall=true", + "-Dmaven.wagon.http.ssl.ignore.validity.dates=true", + "-Daether.connector.https.securityMode=insecure", + ) + } execPath := getMavenExecPath(mdt.useWrapper) //#nosec G204 cmdOutput, err = buildMvnExecCommand(mdt.useWrapper, execPath, goals).CombinedOutput() if err != nil { - stringOutput := string(cmdOutput) + stringOutput := maskCredentials(string(cmdOutput), mdt.server) if len(cmdOutput) > 0 { log.Verbose(stringOutput) } if msg := technologies.GetMsgToUserForCurationBlock(mdt.isCurationCmd, techutils.Maven, stringOutput); msg != "" { err = fmt.Errorf("failed running command '%s %s'\n\n%s", execPath, strings.Join(goals, " "), msg) } else { - err = fmt.Errorf("failed running command '%s %s': %s", execPath, strings.Join(goals, " "), err.Error()) + err = fmt.Errorf("failed running command '%s %s': %w", execPath, strings.Join(goals, " "), err) + if stringOutput != "" { + err = fmt.Errorf("%w\n%s", err, truncateForError(stringOutput)) + } } } return } +// maskCredentials redacts known credentials from output, mirroring uv.go's maskPassword. +func maskCredentials(output string, server *config.ServerDetails) string { + if server == nil { + return output + } + username, password, err := server.GetAuthenticationCredentials() + if err != nil { + return output + } + if password != "" { + output = strings.ReplaceAll(output, password, "***") + } + if username != "" { + output = strings.ReplaceAll(output, username, "***") + } + return output +} + +const maxCapturedOutputInError = 8 * 1024 + +// truncateForError keeps the tail, advanced to a rune boundary to avoid invalid UTF-8. +func truncateForError(output string) string { + if len(output) <= maxCapturedOutputInError { + return output + } + cut := len(output) - maxCapturedOutputInError + for cut < len(output) && !utf8.RuneStart(output[cut]) { + cut++ + } + return fmt.Sprintf("...(truncated %d bytes; see verbose log for full output)...\n%s", cut, output[cut:]) +} + func (mdt *MavenDepTreeManager) GetSettingsXmlPath() string { return mdt.settingsXmlPath } diff --git a/sca/bom/buildinfo/technologies/java/mvn_test.go b/sca/bom/buildinfo/technologies/java/mvn_test.go index ea32f2025..6522f7bd9 100644 --- a/sca/bom/buildinfo/technologies/java/mvn_test.go +++ b/sca/bom/buildinfo/technologies/java/mvn_test.go @@ -1,10 +1,13 @@ package java import ( + "errors" "os" + "os/exec" "path/filepath" "strings" "testing" + "unicode/utf8" "github.com/beevik/etree" "github.com/jfrog/build-info-go/utils" @@ -701,6 +704,7 @@ func TestNewMavenDepTreeManagerPreservesAllParams(t *testing.T) { UseWrapper: true, Server: server, DepsRepo: "test-repo", + InsecureTls: true, IsMavenDepTreeInstalled: true, IsCurationCmd: true, CurationCacheFolder: "/tmp/cache", @@ -720,6 +724,170 @@ func TestNewMavenDepTreeManagerPreservesAllParams(t *testing.T) { assert.Equal(t, "/tmp/cache", manager.curationCacheFolder) assert.Equal(t, Tree, manager.cmdName) assert.True(t, manager.mvnIncludePluginDeps, "MvnIncludePluginDeps must be propagated from params into the manager") + assert.True(t, manager.insecureTls, "InsecureTls must be propagated from params into the manager") +} + +// TestInsecureTlsAddsWagonSslFlags locks in that --insecure-tls reaches the spawned mvn +// process. Before this fix the flag only affected the CLI's own HTTP clients (Xray/Catalog/ +// Artifactory) and had no effect on the internally-invoked mvn subprocess, so a MITM proxy +// that the CLI's own TLS bypass couldn't help with would still fail the plugin resolution. +// Uses a fake mvnw that echoes its args, so no real Maven or network call is needed. +func TestInsecureTlsAddsWagonSslFlags(t *testing.T) { + // No t.Parallel(): this test changes the process-wide working directory. + tmpDir := t.TempDir() + currentDir, err := os.Getwd() + require.NoError(t, err) + restoreDir := tests.ChangeDirWithCallback(t, currentDir, tmpDir) + defer restoreDir() + + require.NoError(t, os.WriteFile("mvnw", []byte("#!/bin/sh\necho \"$@\"\n"), 0644)) + + insecure := NewMavenDepTreeManager(&DepTreeParams{UseWrapper: true, InsecureTls: true}, Tree) + out, err := insecure.RunMvnCmd([]string{"some-goal"}) + require.NoError(t, err) + assert.Contains(t, string(out), "-Dmaven.wagon.http.ssl.insecure=true") + assert.Contains(t, string(out), "-Dmaven.wagon.http.ssl.allowall=true") + assert.Contains(t, string(out), "-Dmaven.wagon.http.ssl.ignore.validity.dates=true") + // Maven 3.9.0+ defaults to the native resolver transport, which ignores the wagon.* properties + // above and reads this one instead — both must be set for --insecure-tls to work regardless of + // which transport the resolved Maven version defaults to. + assert.Contains(t, string(out), "-Daether.connector.https.securityMode=insecure") + + secure := NewMavenDepTreeManager(&DepTreeParams{UseWrapper: true}, Tree) + out, err = secure.RunMvnCmd([]string{"some-goal"}) + require.NoError(t, err) + assert.NotContains(t, string(out), "-Dmaven.wagon.http.ssl") + assert.NotContains(t, string(out), "-Daether.connector.https.securityMode") +} + +// TestRunMvnCmdErrorIncludesCapturedOutput locks in that a failing mvn command's returned +// error contains the process's actual stdout/stderr, not just Go's generic "exit status 1". +// Before this fix that output was only ever logged at the Verbose level (above Debug), so +// even a debug log wouldn't show the real Maven failure reason. +func TestRunMvnCmdErrorIncludesCapturedOutput(t *testing.T) { + // No t.Parallel(): this test changes the process-wide working directory. + tmpDir := t.TempDir() + currentDir, err := os.Getwd() + require.NoError(t, err) + restoreDir := tests.ChangeDirWithCallback(t, currentDir, tmpDir) + defer restoreDir() + + require.NoError(t, os.WriteFile("mvnw", []byte("#!/bin/sh\necho 'a distinctive maven failure marker' >&2\nexit 1\n"), 0644)) + + manager := NewMavenDepTreeManager(&DepTreeParams{UseWrapper: true}, Tree) + _, err = manager.RunMvnCmd([]string{"some-goal"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "a distinctive maven failure marker") +} + +// TestRunMvnCmdErrorTruncatesLargeOutput locks in that embedding captured output into the error +// (added by this same fix) doesn't let an unbounded, noisy Maven run bloat the returned error +// indefinitely. The tail is kept, since a failing run's [ERROR] block is typically at the end. +func TestRunMvnCmdErrorTruncatesLargeOutput(t *testing.T) { + // No t.Parallel(): this test changes the process-wide working directory. + tmpDir := t.TempDir() + currentDir, err := os.Getwd() + require.NoError(t, err) + restoreDir := tests.ChangeDirWithCallback(t, currentDir, tmpDir) + defer restoreDir() + + script := "#!/bin/sh\ni=0\nwhile [ $i -lt 2000 ]; do\n echo \"filler line $i of noisy maven info output\"\n i=$((i+1))\ndone\necho 'a distinctive tail marker'\nexit 1\n" + require.NoError(t, os.WriteFile("mvnw", []byte(script), 0644)) + + manager := NewMavenDepTreeManager(&DepTreeParams{UseWrapper: true}, Tree) + _, err = manager.RunMvnCmd([]string{"some-goal"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "a distinctive tail marker", "the tail of the output, where a real [ERROR] block lives, must survive truncation") + assert.Less(t, len(err.Error()), 2*maxCapturedOutputInError, "error must be bounded even when the underlying command produces a large amount of output") + assert.Contains(t, err.Error(), "truncated", "a truncated error should say so") +} + +// TestTruncateForErrorDoesNotSplitMultiByteRune: the naive byte-count cut lands mid-emoji here. +func TestTruncateForErrorDoesNotSplitMultiByteRune(t *testing.T) { + t.Parallel() + const emoji = "🔥" // 4-byte UTF-8 rune + prefix := strings.Repeat("a", 8) + suffix := strings.Repeat("b", maxCapturedOutputInError-2) + output := prefix + emoji + suffix // naive cut at len(output)-maxCapturedOutputInError == 10, 2 bytes into the emoji + + result := truncateForError(output) + assert.True(t, utf8.ValidString(result), "truncated output must be valid UTF-8, never a dangling continuation byte") +} + +// TestRunMvnCmdErrorNoTrailingNewlineWhenOutputEmpty: no captured output shouldn't leave a dangling "\n". +func TestRunMvnCmdErrorNoTrailingNewlineWhenOutputEmpty(t *testing.T) { + // No t.Parallel(): this test changes the process-wide working directory. + tmpDir := t.TempDir() + currentDir, err := os.Getwd() + require.NoError(t, err) + restoreDir := tests.ChangeDirWithCallback(t, currentDir, tmpDir) + defer restoreDir() + + require.NoError(t, os.WriteFile("mvnw", []byte("#!/bin/sh\nexit 1\n"), 0644)) + + manager := NewMavenDepTreeManager(&DepTreeParams{UseWrapper: true}, Tree) + _, err = manager.RunMvnCmd([]string{"some-goal"}) + require.Error(t, err) + assert.False(t, strings.HasSuffix(err.Error(), "\n"), "error must not have a dangling trailing newline when there is no captured output") +} + +// TestRunMvnCmdMasksCredentialsInError: server credentials must never appear verbatim in the error. +func TestRunMvnCmdMasksCredentialsInError(t *testing.T) { + // No t.Parallel(): this test changes the process-wide working directory. + tmpDir := t.TempDir() + currentDir, err := os.Getwd() + require.NoError(t, err) + restoreDir := tests.ChangeDirWithCallback(t, currentDir, tmpDir) + defer restoreDir() + + script := "#!/bin/sh\necho 'auth failed for user secret-user-42 with password s3cr3t-p4ss!' >&2\nexit 1\n" + require.NoError(t, os.WriteFile("mvnw", []byte(script), 0644)) + + server := &config.ServerDetails{User: "secret-user-42", Password: "s3cr3t-p4ss!"} + manager := NewMavenDepTreeManager(&DepTreeParams{UseWrapper: true, Server: server}, Tree) + _, err = manager.RunMvnCmd([]string{"some-goal"}) + require.Error(t, err) + assert.NotContains(t, err.Error(), "s3cr3t-p4ss!", "password must never appear verbatim in the returned error") + assert.NotContains(t, err.Error(), "secret-user-42", "username must never appear verbatim in the returned error") + assert.Contains(t, err.Error(), "***") +} + +// TestRunMvnCmdErrorWrapsExitError: errors.As must be able to recover the underlying *exec.ExitError. +func TestRunMvnCmdErrorWrapsExitError(t *testing.T) { + // No t.Parallel(): this test changes the process-wide working directory. + tmpDir := t.TempDir() + currentDir, err := os.Getwd() + require.NoError(t, err) + restoreDir := tests.ChangeDirWithCallback(t, currentDir, tmpDir) + defer restoreDir() + + require.NoError(t, os.WriteFile("mvnw", []byte("#!/bin/sh\necho 'some output'\nexit 1\n"), 0644)) + + manager := NewMavenDepTreeManager(&DepTreeParams{UseWrapper: true}, Tree) + _, err = manager.RunMvnCmd([]string{"some-goal"}) + require.Error(t, err) + var exitErr *exec.ExitError + assert.True(t, errors.As(err, &exitErr), "the underlying *exec.ExitError must be recoverable via errors.As") +} + +// TestRunMvnCmdCurationBlockUnaffectedByTruncation: the curation-block branch's fixed message +// must stay independent of the truncation logic in the adjacent branch. +func TestRunMvnCmdCurationBlockUnaffectedByTruncation(t *testing.T) { + // No t.Parallel(): this test changes the process-wide working directory. + tmpDir := t.TempDir() + currentDir, err := os.Getwd() + require.NoError(t, err) + restoreDir := tests.ChangeDirWithCallback(t, currentDir, tmpDir) + defer restoreDir() + + script := "#!/bin/sh\ni=0\nwhile [ $i -lt 2000 ]; do\n echo \"noisy maven output line $i\"\n i=$((i+1))\ndone\necho 'status code: 403'\nexit 1\n" // well over maxCapturedOutputInError + require.NoError(t, os.WriteFile("mvnw", []byte(script), 0644)) + + manager := NewMavenDepTreeManager(&DepTreeParams{UseWrapper: true, IsCurationCmd: true}, Tree) + _, err = manager.RunMvnCmd([]string{"some-goal"}) + require.Error(t, err) + assert.Less(t, len(err.Error()), 500, "the curation-block message is a fixed template, not proportional to output size") + assert.NotContains(t, err.Error(), "truncated", "the curation-block branch must never invoke the truncation logic") } // TestInjectPluginDeps locks in the dedup guard and the module-root fan-out