diff --git a/artifactory/commands/golang/go.go b/artifactory/commands/golang/go.go index 39ac40ae..db7070a9 100644 --- a/artifactory/commands/golang/go.go +++ b/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: //api/go/ 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) { diff --git a/artifactory/commands/golang/go_test.go b/artifactory/commands/golang/go_test.go index 0ffe406e..d0022816 100644 --- a/artifactory/commands/golang/go_test.go +++ b/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) }) diff --git a/artifactory/commands/setup/setup.go b/artifactory/commands/setup/setup.go index 85651aa6..476bf246 100644 --- a/artifactory/commands/setup/setup.go +++ b/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://:@/artifactory/go/,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 } diff --git a/artifactory/commands/setup/setup_test.go b/artifactory/commands/setup/setup_test.go index c02ec8d2..48edac77 100644 --- a/artifactory/commands/setup/setup_test.go +++ b/artifactory/commands/setup/setup_test.go @@ -1,7 +1,9 @@ package setup import ( + "bytes" "fmt" + "io/fs" "net/http" "net/http/httptest" "os" @@ -20,6 +22,7 @@ import ( "github.com/jfrog/jfrog-cli-core/v2/utils/ioutils" "github.com/jfrog/jfrog-client-go/auth" "github.com/jfrog/jfrog-client-go/utils/io" + "github.com/jfrog/jfrog-client-go/utils/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/exp/slices" @@ -91,6 +94,14 @@ func testSetupCommandNpmPnpm(t *testing.T, packageManager project.ProjectType) { // Set NPM_CONFIG_USERCONFIG to point to the temporary npmrc file path. t.Setenv("NPM_CONFIG_USERCONFIG", npmrcFilePath) + // `pnpm config set` ignores NPM_CONFIG_USERCONFIG and writes to its own config + // directory instead, so every variable that locates a home or config directory + // is redirected into tempDir as well. Without this the pnpm cases write into + // the developer's real pnpm configuration and then fail on the missing .npmrc. + t.Setenv("HOME", tempDir) + t.Setenv("USERPROFILE", tempDir) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(tempDir, "xdg")) + t.Setenv("LOCALAPPDATA", filepath.Join(tempDir, "localappdata")) // Set up server details for the current test case's authentication type. loginCmd := createTestSetupCommand(packageManager) @@ -101,10 +112,7 @@ func testSetupCommandNpmPnpm(t *testing.T, packageManager project.ProjectType) { // Run the login command and ensure no errors occur. require.NoError(t, loginCmd.Run()) - // Read the contents of the temporary npmrc file. - npmrcContentBytes, err := os.ReadFile(npmrcFilePath) - assert.NoError(t, err) - npmrcContent := string(npmrcContentBytes) + npmrcContent := readPackageManagerConfigs(t, tempDir) // Validate that the registry URL was set correctly in .npmrc. assert.Contains(t, npmrcContent, fmt.Sprintf("%s=%s", cmdutils.NpmConfigRegistryKey, "https://acme.jfrog.io/artifactory/api/npm/test-repo/")) @@ -118,13 +126,38 @@ func testSetupCommandNpmPnpm(t *testing.T, packageManager project.ProjectType) { expectedBasicAuth := fmt.Sprintf("//acme.jfrog.io/artifactory/api/npm/test-repo/:%s=\"bXlVc2VyOm15UGFzc3dvcmQ=\"", cmdutils.NpmConfigAuthKey) assert.Contains(t, npmrcContent, expectedBasicAuth) } - - // Clean up the temporary npmrc file. - assert.NoError(t, os.Remove(npmrcFilePath)) }) } } +// readPackageManagerConfigs returns the contents of every npm-family configuration file +// written under root. npm writes the file NPM_CONFIG_USERCONFIG names, while pnpm writes +// auth.ini inside its own config directory, whose path differs per platform, so the files +// are found by walking rather than assumed. Only known configuration file names are read, +// to keep caches and log files out of the assertions. +func readPackageManagerConfigs(t *testing.T, root string) string { + configFileNames := []string{".npmrc", "auth.ini", "rc", "config.yaml"} + // The walk only collects paths; the files are read afterwards, so no filesystem + // operation runs inside the callback. + var configPaths []string + require.NoError(t, filepath.WalkDir(root, func(path string, entry fs.DirEntry, err error) error { + if err != nil || entry.IsDir() || !slices.Contains(configFileNames, entry.Name()) { + return err + } + configPaths = append(configPaths, path) + return nil + })) + require.NotEmptyf(t, configPaths, "no package manager configuration was written under %s", root) + + var contents []string + for _, configPath := range configPaths { + content, err := os.ReadFile(configPath) + require.NoError(t, err) + contents = append(contents, string(content)) + } + return strings.Join(contents, "\n") +} + func TestSetupCommand_Yarn(t *testing.T) { // Retrieve the home directory and construct the .yarnrc file path. homeDir, err := os.UserHomeDir() @@ -358,6 +391,12 @@ func TestSetupCommand_Go(t *testing.T) { assert.Contains(t, goProxy, "https://acme.jfrog.io/artifactory/api/go/test-repo") } + // The fallback must be comma-separated. A pipe would make the go command + // fall through to the module's public source on ANY error, including a 403 + // from Artifactory Curation, silently defeating the block. + assert.Contains(t, goProxy, ",direct", "jf setup must limit the direct fallback to 404/410") + assert.NotContains(t, goProxy, "|direct", "a pipe separator would fall back on any error, including a Curation 403") + // Clean up the global GOPROXY setting after each test case err = exec.Command("go", "env", "-u", goProxyEnv).Run() assert.NoError(t, err, "Failed to unset GOPROXY after test case") @@ -923,3 +962,236 @@ func TestSetupCommand_MavenCorrupted(t *testing.T) { assert.NotContains(t, content, testCredential(), "Old token should be replaced") }) } + +// Every supported package manager must describe what it changed: the output is the +// only place a user learns the configuration is user-level rather than scoped to the +// directory they ran the command in. +func TestConfigScopeNote_CoversEverySupportedPackageManager(t *testing.T) { + for _, name := range GetSupportedPackageManagersList() { + packageManager := project.FromString(name) + require.NotEqualf(t, project.ProjectType(-1), packageManager, "%q is not a known project type", name) + + // Clear any override so this asserts the default, user-level wording. + if envVar := packageManagerConfigs[packageManager].overrideEnv; envVar != "" { + t.Setenv(envVar, "") + } + + note := configScopeNote(packageManager) + require.NotEmptyf(t, note, "no scope note for supported package manager %q", name) + + // Each note must take exactly one of the two valid shapes, and a resolution + // note must name the package manager it is talking about. + if packageManagerConfigs[packageManager].credentialsOnly { + assert.Containsf(t, note, "Credentials were saved", "%q: %s", name, note) + assert.NotContainsf(t, note, "applies to every", "%q must not claim resolution: %s", name, note) + continue + } + assert.Containsf(t, note, fmt.Sprintf("applies to every %s project", name), "%q: %s", name, note) + assert.NotContainsf(t, note, "Credentials were saved", "%q: %s", name, note) + } +} + +// A configuration redirected by an environment variable is not user-level — it can sit +// inside the current project — so the note must report that path instead of promising +// a scope that does not hold. +func TestConfigScopeNote_RedirectedConfigDoesNotClaimUserScope(t *testing.T) { + overrides := packageManagersWithConfigOverride() + require.NotEmpty(t, overrides, "expected package managers with a config override") + + for packageManager, envVar := range overrides { + overridePath := filepath.Join(t.TempDir(), "redirected-config") + t.Setenv(envVar, overridePath) + + note := configScopeNote(packageManager) + assert.Containsf(t, note, overridePath, "%s: note should name the redirected path: %s", packageManager.String(), note) + assert.Containsf(t, note, envVar, "%s: note should name the variable that redirected it: %s", packageManager.String(), note) + // The scope claim itself is what must not survive a redirect; the note may still + // mention user-level configuration to contrast against it. + assert.NotContainsf(t, note, "applies to every", "%s must not claim project-wide scope when redirected: %s", packageManager.String(), note) + assert.Containsf(t, note, "scope follows that path", "%s should defer scope to the redirected file: %s", packageManager.String(), note) + } +} + +// With no override set the default user-level wording applies, so the two branches are +// covered in both directions. +func TestConfigScopeNote_WithoutOverrideClaimsUserScope(t *testing.T) { + for packageManager, envVar := range packageManagersWithConfigOverride() { + t.Setenv(envVar, "") + note := configScopeNote(packageManager) + assert.Containsf(t, note, "user-level", "%s: %s", packageManager.String(), note) + assert.NotContainsf(t, note, envVar, "%s: %s", packageManager.String(), note) + } +} + +// Each override was verified against the tool that consumes it, so the set is pinned in +// both directions: a new entry added without that verification fails here, and dropping +// one that works silently downgrades the note to a scope claim that may be wrong. +func TestPackageManagerConfigs_OverridesAreExactlyTheVerifiedSet(t *testing.T) { + expected := map[project.ProjectType]string{ + project.Npm: "NPM_CONFIG_USERCONFIG", + project.Pip: "PIP_CONFIG_FILE", + project.Pipenv: "PIP_CONFIG_FILE", + project.Poetry: "POETRY_CONFIG_DIR", + project.UV: "UV_CONFIG_FILE", + project.Go: "GOENV", + project.Gradle: "GRADLE_USER_HOME", + } + assert.Equal(t, expected, packageManagersWithConfigOverride()) +} + +// pnpm looks like it should follow npm here, and it does not: `pnpm config set` writes to +// pnpm's own config directory and ignores NPM_CONFIG_USERCONFIG (verified against pnpm +// 11 — the file it wrote was auth.ini under the pnpm config directory, both with and +// without the variable set). Claiming the redirect would send users to a file that +// `jf setup pnpm` never touched, so the absence is asserted rather than left to chance. +func TestPackageManagerConfigs_PnpmHasNoConfigOverride(t *testing.T) { + assert.Empty(t, packageManagerConfigs[project.Pnpm].overrideEnv, + "pnpm config set does not honor an environment override") + + customConfig := filepath.Join(t.TempDir(), "custom.npmrc") + t.Setenv("NPM_CONFIG_USERCONFIG", customConfig) + note := configScopeNote(project.Pnpm) + assert.NotContains(t, note, customConfig, "the note must not point at a file pnpm does not write: "+note) + assert.Contains(t, note, "applies to every pnpm project", note) +} + +// packageManagersWithConfigOverride returns only the entries that declare an override +// variable, so the override tests do not have to skip the rest. +func packageManagersWithConfigOverride() map[project.ProjectType]string { + overrides := map[project.ProjectType]string{} + for packageManager, packageManagerConfig := range packageManagerConfigs { + if packageManagerConfig.overrideEnv != "" { + overrides[packageManager] = packageManagerConfig.overrideEnv + } + } + return overrides +} + +// Container logins authenticate rather than redirect resolution, so their note must +// not promise that projects now resolve through Artifactory — an unqualified +// `docker pull alpine` still reaches Docker Hub after `jf setup docker`. +func TestConfigScopeNote_ContainerLoginsDoNotClaimResolution(t *testing.T) { + for _, packageManager := range []project.ProjectType{project.Docker, project.Podman, project.Helm} { + note := configScopeNote(packageManager) + assert.Contains(t, note, "Credentials were saved", packageManager.String()) + assert.NotContains(t, note, "applies to every", packageManager.String()) + } + + // Resolution-changing package managers must state the scope explicitly. + for _, packageManager := range []project.ProjectType{project.Npm, project.Maven, project.Go, project.Pip} { + note := configScopeNote(packageManager) + assert.Contains(t, note, "applies to every", packageManager.String()) + assert.Contains(t, note, "not only the current directory", packageManager.String()) + } +} + +// An unsupported package manager has nothing accurate to say, so it must stay silent +// rather than print a misleading note. +func TestConfigScopeNote_UnknownPackageManagerIsSilent(t *testing.T) { + assert.Empty(t, configScopeNote(project.Cocoapods)) +} + +// The note is only useful if the command actually prints it, so assert the wiring +// rather than just the string builder: removing the log call would otherwise leave +// every configScopeNote test passing. +func TestSetupCommand_PrintsConfigScopeNote(t *testing.T) { + // Maven writes only settings.xml, so a temporary home keeps the run self-contained. + // Both variables are set for cross-platform parity with the other Maven tests. + tempHome := t.TempDir() + t.Setenv("HOME", tempHome) + t.Setenv("USERPROFILE", tempHome) + + var output bytes.Buffer + previousLogger := log.Logger + log.SetLogger(log.NewLogger(log.INFO, &output)) + defer log.SetLogger(previousLogger) + + setupCmd := createTestSetupCommand(project.Maven) + setupCmd.repoName = "test-repo" + require.NoError(t, setupCmd.Run()) + + assert.Contains(t, output.String(), "Successfully configured", "expected the success message") + assert.Contains(t, output.String(), configScopeNote(project.Maven), + "the command must print the scope note, not just be able to build it") +} + +// A pre-existing GOPROXY is echoed back to the user, and GOPROXY is a +// separator-delimited list, so masking has to cover every entry rather than +// stopping at the first set of credentials. +func TestMaskGoProxyCredentials(t *testing.T) { + const tokenOne = "TOKEN_ONE" + const tokenTwo = "TOKEN_TWO" + testCases := []struct { + name string + goProxy string + expected string + }{ + { + name: "Single entry with direct fallback", + goProxy: "https://u:" + tokenOne + "@art.example.com/artifactory/api/go/repo,direct", + expected: "https://****@art.example.com/artifactory/api/go/repo,direct", + }, + { + name: "Comma-separated entries both masked", + goProxy: "https://u:" + tokenOne + "@host1/api/go/r1,https://u:" + tokenTwo + "@host2/api/go/r2", + expected: "https://****@host1/api/go/r1,https://****@host2/api/go/r2", + }, + { + name: "Pipe-separated entries both masked", + goProxy: "https://u:" + tokenOne + "@host1/api/go/r1|https://u:" + tokenTwo + "@host2/api/go/r2", + expected: "https://****@host1/api/go/r1|https://****@host2/api/go/r2", + }, + { + name: "Mixed separators", + goProxy: "https://u:" + tokenOne + "@host1/r1,https://u:" + tokenTwo + "@host2/r2|direct", + expected: "https://****@host1/r1,https://****@host2/r2|direct", + }, + { + name: "Password containing an at sign masks the whole password", + goProxy: "https://user:p@ssw0rd@art.example.com/artifactory/api/go/repo", + expected: "https://****@art.example.com/artifactory/api/go/repo", + }, + { + name: "No credentials is left untouched", + goProxy: "https://proxy.golang.org,direct", + expected: "https://proxy.golang.org,direct", + }, + { + name: "Keywords are left untouched", + goProxy: "off", + expected: "off", + }, + { + name: "Empty value", + goProxy: "", + expected: "", + }, + { + name: "Entry without a scheme still loses its credentials", + goProxy: "u:" + tokenOne + "@host/api/go/repo", + expected: "****@host/api/go/repo", + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + masked := maskGoProxyCredentials(testCase.goProxy) + assert.Equal(t, testCase.expected, masked) + assert.NotContains(t, masked, tokenOne, "no entry's credentials may survive masking") + assert.NotContains(t, masked, tokenTwo, "no entry's credentials may survive masking") + assert.NotContains(t, masked, "ssw0rd", "a password containing '@' must not leak") + }) + } +} + +// packageManagerConfigs drives the note printed after every successful setup, so a +// package manager added to packageManagerToRepositoryPackageType without an entry +// here would silently print nothing. The map comment promises these stay in step. +func TestPackageManagerConfigs_CoversEverySupportedPackageManager(t *testing.T) { + assert.Len(t, packageManagerConfigs, len(packageManagerToRepositoryPackageType)) + for packageManager := range packageManagerToRepositoryPackageType { + config, ok := packageManagerConfigs[packageManager] + if assert.True(t, ok, "%s is supported by jf setup but has no packageManagerConfigs entry", packageManager) { + assert.NotEmpty(t, config.location, "%s has an entry with no location", packageManager) + } + } +}