fix: error sending generate manifest metadata cmp server (#25891)

Signed-off-by: Patroklos Papapetrou <ppapapetrou76@gmail.com>

reposerver/repository/repository.go

@@ -2162,7 +2162,9 @@ func generateManifestsCMP(ctx context.Context, appPath, rootPath string, env []s
 		cmp.WithTarDoneChan(tarDoneCh),
 	}
 
-	err = cmp.SendRepoStream(generateManifestStream.Context(), appPath, rootPath, generateManifestStream, env, tarExcludedGlobs, opts...)
+	// Use the request context (ctx) rather than stream.Context() to avoid prematurely sending into a stream whose
+	// context may have been canceled by the gRPC internals or server-side handling.
+	err = cmp.SendRepoStream(ctx, appPath, rootPath, generateManifestStream, env, tarExcludedGlobs, opts...)
 	if err != nil {
 		return nil, fmt.Errorf("error sending file to cmp-server: %w", err)
 	}
@@ -2384,7 +2386,7 @@ func populatePluginAppDetails(ctx context.Context, res *apiclient.RepoAppDetails
 		return fmt.Errorf("error getting parametersAnnouncementStream: %w", err)
 	}
 
-	err = cmp.SendRepoStream(parametersAnnouncementStream.Context(), appPath, repoPath, parametersAnnouncementStream, env, tarExcludedGlobs)
+	err = cmp.SendRepoStream(ctx, appPath, repoPath, parametersAnnouncementStream, env, tarExcludedGlobs)
 	if err != nil {
 		return fmt.Errorf("error sending file to cmp-server: %w", err)
 	}

util/app/discovery/discovery.go

@@ -92,6 +92,7 @@ func DetectConfigManagementPlugin(ctx context.Context, appPath, repoPath, plugin
 	var conn utilio.Closer
 	var cmpClient pluginclient.ConfigManagementPluginServiceClient
 	var connFound bool
+	var lastErr error
 
 	pluginSockFilePath := common.GetPluginSockFilePath()
 	log.WithFields(log.Fields{
@@ -101,8 +102,11 @@ func DetectConfigManagementPlugin(ctx context.Context, appPath, repoPath, plugin
 
 	if pluginName != "" {
 		// check if the given plugin supports the repo
-		conn, cmpClient, connFound = cmpSupports(ctx, pluginSockFilePath, appPath, repoPath, fmt.Sprintf("%v.sock", pluginName), env, tarExcludedGlobs, true)
+		conn, cmpClient, connFound, lastErr = cmpSupports(ctx, pluginSockFilePath, appPath, repoPath, fmt.Sprintf("%v.sock", pluginName), env, tarExcludedGlobs, true)
 		if !connFound {
+			if lastErr != nil {
+				return nil, nil, fmt.Errorf("could not find cmp-server plugin with name %q supporting the given repository: %w", pluginName, lastErr)
+			}
 			return nil, nil, fmt.Errorf("could not find cmp-server plugin with name %q supporting the given repository", pluginName)
 		}
 	} else {
@@ -112,13 +116,16 @@ func DetectConfigManagementPlugin(ctx context.Context, appPath, repoPath, plugin
 		}
 		for _, file := range fileList {
 			if file.Type() == os.ModeSocket {
-				conn, cmpClient, connFound = cmpSupports(ctx, pluginSockFilePath, appPath, repoPath, file.Name(), env, tarExcludedGlobs, false)
+				conn, cmpClient, connFound, lastErr = cmpSupports(ctx, pluginSockFilePath, appPath, repoPath, file.Name(), env, tarExcludedGlobs, false)
 				if connFound {
 					break
 				}
 			}
 		}
 		if !connFound {
+			if lastErr != nil {
+				return nil, nil, fmt.Errorf("could not find plugin supporting the given repository: %w", lastErr)
+			}
 			return nil, nil, errors.New("could not find plugin supporting the given repository")
 		}
 	}
@@ -145,16 +152,20 @@ func matchRepositoryCMP(ctx context.Context, appPath, repoPath string, client pl
 	return resp.GetIsSupported(), resp.GetIsDiscoveryEnabled(), nil
 }
 
-func cmpSupports(ctx context.Context, pluginSockFilePath, appPath, repoPath, fileName string, env []string, tarExcludedGlobs []string, namedPlugin bool) (utilio.Closer, pluginclient.ConfigManagementPluginServiceClient, bool) {
+// cmpSupports checks if the given plugin socket supports the repo. If namedPlugin is true
+// a plugin that is not discovery-configured will be considered a match.
+// Returns a utilio.Closer, the client, a bool indicating a match, and an error describing
+// any lower-level failure that occurred while probing the plugin.
+func cmpSupports(ctx context.Context, pluginSockFilePath, appPath, repoPath, fileName string, env []string, tarExcludedGlobs []string, namedPlugin bool) (utilio.Closer, pluginclient.ConfigManagementPluginServiceClient, bool, error) {
 	absPluginSockFilePath, err := filepath.Abs(pluginSockFilePath)
 	if err != nil {
 		log.Errorf("error getting absolute path for plugin socket dir %v, %v", pluginSockFilePath, err)
-		return nil, nil, false
+		return nil, nil, false, fmt.Errorf("error getting absolute path for plugin socket dir %q: %w", pluginSockFilePath, err)
 	}
 	address := filepath.Join(absPluginSockFilePath, fileName)
 	if !files.Inbound(address, absPluginSockFilePath) {
 		log.Errorf("invalid socket file path, %v is outside plugin socket dir %v", fileName, pluginSockFilePath)
-		return nil, nil, false
+		return nil, nil, false, fmt.Errorf("invalid socket file path, %q is outside plugin socket dir %q", fileName, pluginSockFilePath)
 	}
 
 	cmpclientset := pluginclient.NewConfigManagementPluginClientSet(address)
@@ -165,23 +176,23 @@ func cmpSupports(ctx context.Context, pluginSockFilePath, appPath, repoPath, fil
 			common.SecurityField:    common.SecurityMedium,
 			common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor,
 		}).Errorf("error dialing to cmp-server for plugin %s, %v", fileName, err)
-		return nil, nil, false
+		return nil, nil, false, fmt.Errorf("error dialing to cmp-server for plugin %s: %w", fileName, err)
 	}
 
 	cfg, err := cmpClient.CheckPluginConfiguration(ctx, &empty.Empty{})
 	if err != nil {
 		log.Errorf("error checking plugin configuration %s, %v", fileName, err)
 		utilio.Close(conn)
-		return nil, nil, false
+		return nil, nil, false, fmt.Errorf("error checking plugin configuration %s: %w", fileName, err)
 	}
 
 	if !cfg.IsDiscoveryConfigured {
 		// If discovery isn't configured but the plugin is named, then the plugin supports the repo.
 		if namedPlugin {
-			return conn, cmpClient, true
+			return conn, cmpClient, true, nil
 		}
 		utilio.Close(conn)
-		return nil, nil, false
+		return nil, nil, false, nil
 	}
 
 	isSupported, isDiscoveryEnabled, err := matchRepositoryCMP(ctx, appPath, repoPath, cmpClient, env, tarExcludedGlobs)
@@ -191,20 +202,20 @@ func cmpSupports(ctx context.Context, pluginSockFilePath, appPath, repoPath, fil
 			common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor,
 		}).Errorf("repository %s is not the match because %v", repoPath, err)
 		utilio.Close(conn)
-		return nil, nil, false
+		return nil, nil, false, fmt.Errorf("error checking repository support for plugin %s: %w", fileName, err)
 	}
 
 	if !isSupported {
 		// if discovery is not set and the plugin name is specified, let app use the plugin
 		if !isDiscoveryEnabled && namedPlugin {
-			return conn, cmpClient, true
+			return conn, cmpClient, true, nil
 		}
 		log.WithFields(log.Fields{
 			common.SecurityField:    common.SecurityLow,
 			common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor,
 		}).Debugf("Response from socket file %s does not support %v", fileName, repoPath)
 		utilio.Close(conn)
-		return nil, nil, false
+		return nil, nil, false, nil
 	}
-	return conn, cmpClient, true
+	return conn, cmpClient, true, nil
 }

util/app/discovery/discovery_test.go

@@ -1,6 +1,7 @@
 package discovery
 
 import (
+	"context"
 	"testing"
 
 	"github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
@@ -49,3 +50,35 @@ func TestAppType_Disabled(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, "Directory", appType)
 }
+
+func Test_cmpSupports_invalidSocketPath_outsideDir(t *testing.T) {
+	// Use a temp dir as the base plugin socket dir and provide a fileName that
+	// resolves outside it to trigger the Inbound check.
+	pluginSockFilePath := t.TempDir()
+	// fileName with a path traversal that causes the address to be outside the plugin socket dir
+	fileName := "../outside.sock"
+
+	conn, client, found, err := cmpSupports(context.Background(), pluginSockFilePath, "appPath", "repoPath", fileName, nil, nil, true)
+	require.False(t, found)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "outside plugin socket dir")
+	assert.Nil(t, conn)
+	assert.Nil(t, client)
+}
+
+func Test_cmpSupports_dialFailure_returnsError(t *testing.T) {
+	// Use a temp dir as the base plugin socket dir and provide a socket filename
+	// that does not have a listening server; dialing should fail, and the error
+	// returned should reflect a dialing problem.
+	pluginSockFilePath := t.TempDir()
+	fileName := "nonexistent.sock"
+
+	conn, client, found, err := cmpSupports(context.Background(), pluginSockFilePath, "appPath", "repoPath", fileName, nil, nil, true)
+	require.False(t, found)
+	require.Error(t, err)
+	// We expect the error to at least indicate dialing failure; exact wording may vary,
+	// check for the dialing error prefix used in cmpSupports.
+	assert.Contains(t, err.Error(), "error dialing to cmp-server")
+	assert.Nil(t, conn)
+	assert.Nil(t, client)
+}

util/cmp/stream.go

@@ -96,6 +96,10 @@ func SendRepoStream(ctx context.Context, appPath, rootPath string, sender Stream
 	defer tgzstream.CloseAndDelete(tgz)
 	err = sender.Send(mr)
 	if err != nil {
+		// include ctx.Err() in the message to make cancellations/deadlines visible
+		if ctx != nil && ctx.Err() != nil {
+			return fmt.Errorf("error sending generate manifest metadata to cmp-server: %w (stream ctx err: %w)", err, ctx.Err())
+		}
 		return fmt.Errorf("error sending generate manifest metadata to cmp-server: %w", err)
 	}