Limit jf setup go GOPROXY fallback to 404/410, state which configuration jf setup changed, and mask every GOPROXY entry

artifactory/commands/golang/go.go

@@ -356,12 +356,27 @@ func logGoVersion() error {
 	return nil
 }
 
+// The "direct" GOPROXY entry tells the go command to fetch the module from its own
+// source. The separator in front of it decides when that happens: a pipe falls back
+// on any proxy error, a comma only on 404/410 - so with a comma a Curation 403 is a
+// hard failure instead of being quietly satisfied from the module's public source.
+const (
+	directFallbackEntry           = "direct"
+	anyErrorFallbackSeparator     = "|"
+	notFoundOnlyFallbackSeparator = ","
+)
+
 type GoProxyUrlParams struct {
 	// Fallback to retrieve the modules directly from the source if
 	// the module failed to be retrieved from the proxy.
 	// add |direct to the end of the url.
 	// example: https://gocenter.io|direct
 	Direct bool
+	// FallbackOnlyIfNotFound joins the "direct" entry with a comma instead of a
+	// pipe, so the go command falls through to the module's public source only
+	// after a 404/410. The zero value keeps the previous any-error (pipe)
+	// behavior for existing callers. Ignored when Direct is false.
+	FallbackOnlyIfNotFound bool
 	// The path from baseUrl to the standard Go repository path
 	// URL structure: <baseUrl>/<EndpointPrefix>/api/go/<repoName>
 	EndpointPrefix string
@@ -374,10 +389,20 @@ func (gdu *GoProxyUrlParams) BuildUrl(url *url.URL, repoName string) string {
 }
 
 func (gdu *GoProxyUrlParams) addDirect(url string) string {
-	if gdu.Direct && !strings.HasSuffix(url, "|direct") {
-		return url + "|direct"
+	if !gdu.Direct || hasDirectFallback(url) {
+		return url
+	}
+	if gdu.FallbackOnlyIfNotFound {
+		return url + notFoundOnlyFallbackSeparator + directFallbackEntry
 	}
-	return url
+	return url + anyErrorFallbackSeparator + directFallbackEntry
+}
+
+// hasDirectFallback reports whether the url already ends with a "direct" entry, with
+// either separator, so that an already-configured fallback is never appended twice.
+func hasDirectFallback(url string) bool {
+	return strings.HasSuffix(url, anyErrorFallbackSeparator+directFallbackEntry) ||
+		strings.HasSuffix(url, notFoundOnlyFallbackSeparator+directFallbackEntry)
 }
 
 func GetArtifactoryRemoteRepoUrl(serverDetails *config.ServerDetails, repo string, goProxyParams GoProxyUrlParams) (string, error) {

artifactory/commands/golang/go_test.go

@@ -93,6 +93,19 @@ func TestGetArtifactoryRemoteRepoUrl(t *testing.T) {
 	repoUrl, err := GetArtifactoryRemoteRepoUrl(server, repoName, GoProxyUrlParams{})
 	assert.NoError(t, err)
 	assert.Equal(t, "https://testuser:"+testFakeToken+"@server.com/artifactory/api/go/test-repo", repoUrl)
+
+	// The exact parameters `jf setup go` passes. Asserted here because the only
+	// other coverage of this shape is TestSetupCommand_Go, which needs the `go`
+	// binary and writes a global env var, so it cannot run everywhere.
+	repoUrl, err = GetArtifactoryRemoteRepoUrl(server, repoName, GoProxyUrlParams{Direct: true, FallbackOnlyIfNotFound: true})
+	assert.NoError(t, err)
+	assert.Equal(t, "https://testuser:"+testFakeToken+"@server.com/artifactory/api/go/test-repo,direct", repoUrl)
+	assert.NotContains(t, repoUrl, "|direct", "a pipe would fall back on any error, including a Curation 403")
+
+	// The parameters `jf go` passes must keep falling back on any error.
+	repoUrl, err = GetArtifactoryRemoteRepoUrl(server, repoName, GoProxyUrlParams{Direct: true})
+	assert.NoError(t, err)
+	assert.Equal(t, "https://testuser:"+testFakeToken+"@server.com/artifactory/api/go/test-repo|direct", repoUrl)
 }
 
 func TestGetArtifactoryApiUrl(t *testing.T) {
@@ -117,6 +130,12 @@ func TestGetArtifactoryApiUrl(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, "https://frog:testpass@test.com/artifactory/test/api/go/test-repo|direct", url)
 
+	// Same, but with the fallback limited to 404/410 - the credentials must still
+	// be embedded and the comma must survive alongside the endpoint prefix.
+	url, err = getArtifactoryApiUrl("test-repo", details, GoProxyUrlParams{EndpointPrefix: "test", Direct: true, FallbackOnlyIfNotFound: true})
+	assert.NoError(t, err)
+	assert.Equal(t, "https://frog:testpass@test.com/artifactory/test/api/go/test-repo,direct", url)
+
 	// Test access token
 	// Set fake access token with username "test"
 	details.SetUser("testuser")
@@ -135,13 +154,33 @@ func TestGetArtifactoryApiUrl(t *testing.T) {
 	assert.Equal(t, "https://frog:"+testFakeToken+"@test.com/artifactory/api/go/test-repo", url)
 }
 
+func TestGoProxyUrlParams_AddDirectIsIdempotentForBothSeparators(t *testing.T) {
+	testCases := []struct {
+		name                   string
+		url                    string
+		FallbackOnlyIfNotFound bool
+	}{
+		{name: "Pipe fallback is not appended twice", url: "https://test/api/go/go|direct"},
+		{name: "Comma fallback is not appended twice", url: "https://test/api/go/go,direct", FallbackOnlyIfNotFound: true},
+		{name: "Comma fallback is left alone when a pipe was requested", url: "https://test/api/go/go,direct"},
+		{name: "Pipe fallback is left alone when a comma was requested", url: "https://test/api/go/go|direct", FallbackOnlyIfNotFound: true},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			gdu := &GoProxyUrlParams{Direct: true, FallbackOnlyIfNotFound: testCase.FallbackOnlyIfNotFound}
+			assert.Equal(t, testCase.url, gdu.addDirect(testCase.url))
+		})
+	}
+}
+
 func TestGoProxyUrlParams_BuildUrl(t *testing.T) {
 	testCases := []struct {
-		name           string
-		RepoName       string
-		Direct         bool
-		EndpointPrefix string
-		ExpectedUrl    string
+		name                   string
+		RepoName               string
+		Direct                 bool
+		FallbackOnlyIfNotFound bool
+		EndpointPrefix         string
+		ExpectedUrl            string
 	}{
 		{
 			name:        "Url Without direct or Prefix",
@@ -160,14 +199,43 @@ func TestGoProxyUrlParams_BuildUrl(t *testing.T) {
 			EndpointPrefix: "prefix",
 			ExpectedUrl:    "https://test/prefix/api/go/go",
 		},
+		{
+			name:                   "Zero value keeps the any-error pipe for existing callers",
+			RepoName:               "go",
+			Direct:                 true,
+			FallbackOnlyIfNotFound: false,
+			ExpectedUrl:            "https://test/api/go/go|direct",
+		},
+		{
+			name:                   "FallbackOnlyIfNotFound limits the fallback to 404/410",
+			RepoName:               "go",
+			Direct:                 true,
+			FallbackOnlyIfNotFound: true,
+			ExpectedUrl:            "https://test/api/go/go,direct",
+		},
+		{
+			name:                   "Ignored when Direct is false",
+			RepoName:               "go",
+			FallbackOnlyIfNotFound: true,
+			ExpectedUrl:            "https://test/api/go/go",
+		},
+		{
+			name:                   "Comma fallback together with an endpoint prefix",
+			RepoName:               "go",
+			Direct:                 true,
+			FallbackOnlyIfNotFound: true,
+			EndpointPrefix:         "prefix",
+			ExpectedUrl:            "https://test/prefix/api/go/go,direct",
+		},
 	}
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
 			remoteUrl, err := url.Parse("https://test")
 			require.NoError(t, err)
 			gdu := &GoProxyUrlParams{
-				Direct:         testCase.Direct,
-				EndpointPrefix: testCase.EndpointPrefix,
+				Direct:                 testCase.Direct,
+				FallbackOnlyIfNotFound: testCase.FallbackOnlyIfNotFound,
+				EndpointPrefix:         testCase.EndpointPrefix,
 			}
 			assert.Equalf(t, testCase.ExpectedUrl, gdu.BuildUrl(remoteUrl, testCase.RepoName), "BuildUrl(%v, %v)", remoteUrl, testCase.RepoName)
 		})

artifactory/commands/setup/setup.go

@@ -33,6 +33,93 @@ import (
 	"golang.org/x/exp/maps"
 )
 
+// packageManagerConfig describes the configuration `jf setup` writes for one package
+// manager, so the command can say what it changed and how far the change reaches.
+// Saying so matters because nothing else in the output reveals that the change is not
+// scoped to the directory the command was run in.
+type packageManagerConfig struct {
+	// location names the configuration in the user's terms rather than as an absolute
+	// path: the real path is platform dependent (pip alone has three per-user
+	// locations) and the package manager resolves it itself, so a path spelled out
+	// here would be wrong on some machines.
+	location string
+	// credentialsOnly marks the package managers that only store credentials instead
+	// of redirecting resolution. Their projects do not start resolving through
+	// Artifactory — an unqualified `docker pull alpine` still goes to Docker Hub — so
+	// the note must not claim otherwise.
+	credentialsOnly bool
+	// overrideEnv is the environment variable that moves this configuration off its
+	// user-level default. When it is set the configuration can live anywhere the user
+	// pointed it, including inside the current project, so the note has to describe
+	// that path instead of claiming user-wide scope. Only set where the configure
+	// function or the tool it drives really honors the variable — the per-entry
+	// comments record what was verified.
+	overrideEnv string
+}
+
+// One entry per package manager in packageManagerToRepositoryPackageType;
+// TestPackageManagerConfigs_CoversEverySupportedPackageManager asserts the two
+// stay in step.
+var packageManagerConfigs = map[project.ProjectType]packageManagerConfig{
+	// npm resolves the file `npm config set` writes through NPM_CONFIG_USERCONFIG.
+	project.Npm: {location: "your user-level npm configuration (.npmrc)", overrideEnv: "NPM_CONFIG_USERCONFIG"},
+	// pnpm is deliberately left without an override: `pnpm config set` writes to
+	// pnpm's own config directory (auth.ini) and ignores NPM_CONFIG_USERCONFIG, which
+	// it consults only as a fallback when reading credentials.
+	// That file also outranks ~/.npmrc for pnpm, which matters across two setups: a
+	// machine with no pnpm setup inherits npm's ~/.npmrc through the fallback, but
+	// once `jf setup pnpm` has written auth.ini, a later `jf setup npm` moves npm
+	// alone and pnpm keeps resolving from the repository it was given. Both tools are
+	// individually correct, and nothing in either command's output says they now
+	// disagree. Verified against pnpm 11.
+	project.Pnpm: {location: "your user-level pnpm configuration (auth.ini in pnpm's config directory)"},
+	// Yarn Classic writes ~/.yarnrc, and YARN_RC_FILENAME does not redirect it.
+	project.Yarn: {location: "your user-level Yarn configuration (.yarnrc)"},
+	// configurePip writes the file itself when PIP_CONFIG_FILE is set, because
+	// `pip config set` does not support that variable.
+	project.Pip:    {location: "your user-level pip configuration (pip.conf)", overrideEnv: "PIP_CONFIG_FILE"},
+	project.Pipenv: {location: "your user-level pip configuration (pip.conf)", overrideEnv: "PIP_CONFIG_FILE"},
+	// `poetry config` writes config.toml into POETRY_CONFIG_DIR when it is set.
+	project.Poetry: {location: "your user-level Poetry configuration (config.toml)", overrideEnv: "POETRY_CONFIG_DIR"},
+	// Twine's .pypirc path is chosen per invocation (--config-file), not by the environment.
+	project.Twine: {location: "your user-level Twine configuration (.pypirc)"},
+	// ConfigureUVIndex writes to UV_CONFIG_FILE when it is set.
+	project.UV:     {location: "your user-level uv configuration (uv.toml)", overrideEnv: "UV_CONFIG_FILE"},
+	project.Nuget:  {location: "your user-level NuGet configuration (NuGet.Config)"},
+	project.Dotnet: {location: "your user-level NuGet configuration (NuGet.Config)"},
+	// `go env -w` writes to the file GOENV points at, defaulting to the per-user Go env file.
+	project.Go: {location: "your user-level Go environment (GOPROXY in your Go env file)", overrideEnv: "GOENV"},
+	// gradle.WriteInitScript drops the script under GRADLE_USER_HOME when it is set.
+	project.Gradle: {location: "your user-level Gradle configuration (an init script in your Gradle user home)", overrideEnv: gradle.UserHomeEnv},
+	// Maven picks the settings file per invocation (-s), not from the environment.
+	project.Maven:  {location: "your user-level Maven settings (settings.xml)"},
+	project.Docker: {location: "your Docker credential store", credentialsOnly: true},
+	project.Podman: {location: "your Podman credential store", credentialsOnly: true},
+	project.Helm:   {location: "your Helm registry credential store", credentialsOnly: true},
+}
+
+// configScopeNote describes what the command changed and how widely it applies, or
+// an empty string for a package manager we have nothing accurate to say about.
+func configScopeNote(packageManager project.ProjectType) string {
+	packageManagerConfig, ok := packageManagerConfigs[packageManager]
+	if !ok {
+		return ""
+	}
+	if packageManagerConfig.credentialsOnly {
+		return fmt.Sprintf("Credentials were saved to %s for your user account.", packageManagerConfig.location)
+	}
+	// A redirected configuration is not user-level, so report where it actually went
+	// rather than promising a scope that may not hold.
+	if packageManagerConfig.overrideEnv != "" {
+		if overridePath := os.Getenv(packageManagerConfig.overrideEnv); overridePath != "" {
+			return fmt.Sprintf("This updated the %s configuration at %s, because %s is set, so its scope follows that path rather than your user-level configuration.",
+				packageManager.String(), overridePath, packageManagerConfig.overrideEnv)
+		}
+	}
+	return fmt.Sprintf("This updated %s, so it applies to every %s project for this user, not only the current directory.",
+		packageManagerConfig.location, packageManager.String())
+}
+
 // packageManagerToRepositoryPackageType maps project types to corresponding Artifactory repository package types.
 var packageManagerToRepositoryPackageType = map[project.ProjectType]string{
 	// Npm package managers
@@ -195,6 +282,9 @@ func (sc *SetupCommand) Run() (err error) {
 		repoPrefix = coreutils.PrintBoldTitle(fmt.Sprintf(" repository '%s'", sc.repoName))
 	}
 	log.Output(fmt.Sprintf("Successfully configured %s to use JFrog%s.", coreutils.PrintBoldTitle(sc.packageManager.String()), repoPrefix))
+	if note := configScopeNote(sc.packageManager); note != "" {
+		log.Output(note)
+	}
 	return nil
 }
 
@@ -348,31 +438,81 @@ func (sc *SetupCommand) configureYarn() (err error) {
 	return nil
 }
 
+// goProxySeparators are the two characters that delimit GOPROXY entries: a comma
+// falls through only on 404/410, a pipe on any error.
+const goProxySeparators = ",|"
+
+// maskGoProxyCredentials replaces the credentials of every entry in a GOPROXY
+// value, keeping the scheme and host so the message still says which proxy is set.
+//
+// GOPROXY is a separator-delimited list, so masking only up to the first '@'
+// would print every later entry's token verbatim. Within one entry the LAST '@'
+// delimits the credentials, because a password may itself contain '@'.
+func maskGoProxyCredentials(goProxy string) string {
+	var masked strings.Builder
+	entryStart := 0
+	for i, char := range goProxy {
+		if !strings.ContainsRune(goProxySeparators, char) {
+			continue
+		}
+		masked.WriteString(maskGoProxyEntry(goProxy[entryStart:i]))
+		masked.WriteRune(char)
+		entryStart = i + len(string(char))
+	}
+	masked.WriteString(maskGoProxyEntry(goProxy[entryStart:]))
+	return masked.String()
+}
+
+// maskGoProxyEntry masks the credentials of a single GOPROXY entry. Entries
+// without credentials — including the bare `direct` and `off` keywords — are
+// returned unchanged.
+func maskGoProxyEntry(entry string) string {
+	credentialsEnd := strings.LastIndex(entry, "@")
+	if credentialsEnd == -1 {
+		return entry
+	}
+	scheme := ""
+	if schemeEnd := strings.Index(entry, "://"); schemeEnd != -1 && schemeEnd < credentialsEnd {
+		scheme = entry[:schemeEnd+len("://")]
+	}
+	return scheme + "****" + entry[credentialsEnd:]
+}
+
 // configureGo configures Go to use the Artifactory repository for GOPROXY.
 // Runs the following command:
 //
 //	go env -w GOPROXY=https://<user>:<token>@<your-artifactory-url>/artifactory/go/<repo-name>,direct
+//
+// The comma is deliberate. Unlike `jf go`, which resolves through the CLI for a
+// single invocation and exposes --no-fallback, this writes a persistent global
+// GOPROXY consumed by the native go command with no opt-out. A comma limits the
+// fallback to 404/410 (module not proxied); a pipe would fall through on ANY
+// error, so a 403 from Artifactory Curation would be silently satisfied from the
+// module's public source.
 func (sc *SetupCommand) configureGo() error {
 	if goProxyVal := os.Getenv("GOPROXY"); goProxyVal != "" {
 		// Remove the variable so it won't override the newly configured proxy (temporarily).
 		if err := os.Unsetenv("GOPROXY"); err != nil {
 			return errorutils.CheckErrorf("failed to unset GOPROXY environment variable: %w", err)
 		}
-		// Mask credentials in the GOPROXY value
-		if i := strings.Index(goProxyVal, "@"); i != -1 {
-			goProxyVal = "****" + goProxyVal[i:]
-		}
 		// Log a warning about the existing GOPROXY environment variable so the user can unset it permanently
 		log.Warn(fmt.Sprintf("A local GOPROXY='%s' is set and will override the global setting.\n"+
-			"Unset it in your shell config (e.g., .zshrc, .bashrc).", goProxyVal))
+			"Unset it in your shell config (e.g., .zshrc, .bashrc).", maskGoProxyCredentials(goProxyVal)))
 	}
-	repoWithCredsUrl, err := golang.GetArtifactoryRemoteRepoUrl(sc.serverDetails, sc.repoName, golang.GoProxyUrlParams{Direct: true})
+	repoWithCredsUrl, err := golang.GetArtifactoryRemoteRepoUrl(sc.serverDetails, sc.repoName,
+		golang.GoProxyUrlParams{Direct: true, FallbackOnlyIfNotFound: true})
 	if err != nil {
 		return fmt.Errorf("failed to get Go repository URL: %w", err)
 	}
 	if err := biutils.RunGo([]string{"env", "-w", "GOPROXY=" + repoWithCredsUrl}, ""); err != nil {
 		return fmt.Errorf("failed to set GOPROXY environment variable: %w", err)
 	}
+	// This is a behavior change worth surfacing: previously any proxy error fell
+	// back to the module's public source, so an unreachable Artifactory still
+	// produced a working build.
+	log.Info("GOPROXY falls back to the module's source only for modules the repository does not serve (404/410). " +
+		"Any other error, including a Curation block or an unreachable Artifactory, now fails the command instead of " +
+		"resolving from the public internet.")
 	return nil
 }