From 6764f65247bb3c1c82ad5d8d3d331ba21f7c4a97 Mon Sep 17 00:00:00 2001 From: Brian Gardiner Date: Wed, 29 Apr 2026 10:19:46 -0400 Subject: [PATCH 1/4] feat: introduce SNYK_REQUEST_CONCURRENCY for dependency request parallelism Add a tunable concurrency knob for in-flight dependency-test/dependency-monitor HTTP requests, default 10 (raised from the prior hard-coded 5), clamped to [1, 50]. Override via the SNYK_REQUEST_CONCURRENCY environment variable. Apply the new helper at the existing pMap call site in sendAndParseResults (run-test.ts). The default bump is effectively a no-op for non-container test workloads (single-project tests produce one payload; --all-projects rarely produces more than the prior 5-payload ceiling), but materially improves wall-clock for container tests that produce one ScanResult per directory containing dependencies. A follow-up PR will adopt the same helper in the container monitor path, which is currently fully sequential. --- src/lib/snyk-test/common.ts | 22 ++++++++ src/lib/snyk-test/run-test.ts | 6 +- test/jest/unit/lib/snyk-test/common.spec.ts | 62 ++++++++++++++++++++- 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/src/lib/snyk-test/common.ts b/src/lib/snyk-test/common.ts index 73dcf723cd..3116397e4f 100644 --- a/src/lib/snyk-test/common.ts +++ b/src/lib/snyk-test/common.ts @@ -111,6 +111,28 @@ export type FailOn = 'all' | 'upgradable' | 'patchable'; export const RETRY_ATTEMPTS = 3; export const RETRY_DELAY = 500; +const DEFAULT_REQUEST_CONCURRENCY = 10; +const MIN_REQUEST_CONCURRENCY = 1; +const MAX_REQUEST_CONCURRENCY = 50; + +/** + * Returns the maximum number of in-flight Snyk dependency-test or + * dependency-monitor HTTP requests permitted at once. Override with the + * SNYK_REQUEST_CONCURRENCY environment variable; values are clamped to + * [MIN_REQUEST_CONCURRENCY, MAX_REQUEST_CONCURRENCY]. + */ +export function getRequestConcurrency(): number { + const raw = process.env.SNYK_REQUEST_CONCURRENCY; + if (!raw) { + return DEFAULT_REQUEST_CONCURRENCY; + } + const parsed = parseInt(raw, 10); + if (!Number.isFinite(parsed) || parsed < MIN_REQUEST_CONCURRENCY) { + return DEFAULT_REQUEST_CONCURRENCY; + } + return Math.min(parsed, MAX_REQUEST_CONCURRENCY); +} + /** * printDepGraph writes the given dep-graph and target name to the destination * stream as expected by the `depgraph` CLI workflow. diff --git a/src/lib/snyk-test/run-test.ts b/src/lib/snyk-test/run-test.ts index 7be926b564..7210d73129 100644 --- a/src/lib/snyk-test/run-test.ts +++ b/src/lib/snyk-test/run-test.ts @@ -39,6 +39,7 @@ import { isCI } from '../is-ci'; import { RETRY_ATTEMPTS, RETRY_DELAY, + getRequestConcurrency, printDepGraph, printEffectiveDepGraph, printEffectiveDepGraphError, @@ -94,9 +95,6 @@ import { ProblemError } from '@snyk/error-catalog-nodejs-public'; const debug = debugModule('snyk:run-test'); -// Controls the number of simultaneous test requests that can be in-flight. -const MAX_CONCURRENCY = 5; - function prepareResponseForParsing( payload: Payload, response: TestDependenciesResponse, @@ -293,7 +291,7 @@ async function sendAndParseResults( }; const responses = await pMap(payloads, sendRequest, { - concurrency: MAX_CONCURRENCY, + concurrency: getRequestConcurrency(), }); for (const { payload, originalPayload, response } of responses) { diff --git a/test/jest/unit/lib/snyk-test/common.spec.ts b/test/jest/unit/lib/snyk-test/common.spec.ts index 5afe8cad37..4df25d4c4e 100644 --- a/test/jest/unit/lib/snyk-test/common.spec.ts +++ b/test/jest/unit/lib/snyk-test/common.spec.ts @@ -1,7 +1,10 @@ import { CLI, ProblemError } from '@snyk/error-catalog-nodejs-public'; import { CustomError } from '../../../../../src/lib/errors'; import { FailedProjectScanError } from '../../../../../src/lib/plugins/get-multi-plugin-result'; -import { getOrCreateErrorCatalogError } from '../../../../../src/lib/snyk-test/common'; +import { + getOrCreateErrorCatalogError, + getRequestConcurrency, +} from '../../../../../src/lib/snyk-test/common'; describe('getOrCreateErrorCatalogError', () => { const defaultErrMessage = 'Default error message'; @@ -116,3 +119,60 @@ describe('getOrCreateErrorCatalogError', () => { expect(result.detail).toBe(defaultErrMessage); }); }); + +describe('getRequestConcurrency', () => { + const originalValue = process.env.SNYK_REQUEST_CONCURRENCY; + + afterEach(() => { + if (originalValue === undefined) { + delete process.env.SNYK_REQUEST_CONCURRENCY; + } else { + process.env.SNYK_REQUEST_CONCURRENCY = originalValue; + } + }); + + it('returns the default of 10 when SNYK_REQUEST_CONCURRENCY is unset', () => { + delete process.env.SNYK_REQUEST_CONCURRENCY; + expect(getRequestConcurrency()).toBe(10); + }); + + it('returns the default of 10 when SNYK_REQUEST_CONCURRENCY is empty', () => { + process.env.SNYK_REQUEST_CONCURRENCY = ''; + expect(getRequestConcurrency()).toBe(10); + }); + + it('returns the parsed value when SNYK_REQUEST_CONCURRENCY is a valid integer', () => { + process.env.SNYK_REQUEST_CONCURRENCY = '15'; + expect(getRequestConcurrency()).toBe(15); + }); + + it('clamps to the maximum of 50 when SNYK_REQUEST_CONCURRENCY exceeds the cap', () => { + process.env.SNYK_REQUEST_CONCURRENCY = '500'; + expect(getRequestConcurrency()).toBe(50); + }); + + it('returns the default when SNYK_REQUEST_CONCURRENCY is below the minimum', () => { + process.env.SNYK_REQUEST_CONCURRENCY = '0'; + expect(getRequestConcurrency()).toBe(10); + }); + + it('returns the default when SNYK_REQUEST_CONCURRENCY is negative', () => { + process.env.SNYK_REQUEST_CONCURRENCY = '-5'; + expect(getRequestConcurrency()).toBe(10); + }); + + it('returns the default when SNYK_REQUEST_CONCURRENCY is non-numeric', () => { + process.env.SNYK_REQUEST_CONCURRENCY = 'abc'; + expect(getRequestConcurrency()).toBe(10); + }); + + it('honors the minimum boundary', () => { + process.env.SNYK_REQUEST_CONCURRENCY = '1'; + expect(getRequestConcurrency()).toBe(1); + }); + + it('honors the maximum boundary', () => { + process.env.SNYK_REQUEST_CONCURRENCY = '50'; + expect(getRequestConcurrency()).toBe(50); + }); +}); From f815d2088105e7b1dbc089fa204ce2824e4490ad Mon Sep 17 00:00:00 2001 From: Brian Gardiner Date: Wed, 13 May 2026 10:22:33 -0400 Subject: [PATCH 2/4] feat: route SNYK_REQUEST_CONCURRENCY through GAF, default 5 Address review feedback on the test/monitor request-concurrency knob: - Restore the default to 5 (the prior hard-coded value), so the env-var introduction is purely a configurability change. The default-bump question can be revisited separately once we have telemetry, per Peter's review. - Make the GAF configuration the single source of truth for the user-facing SNYK_REQUEST_CONCURRENCY value: register a new cliv2.ConfigKeyRequestConcurrency key, with snyk_request_concurrency as an alternative key (so the env var feeds the config). The Go side forwards the resolved value to the legacy CLI process via the internal SNYK_INTERNAL_REQUEST_CONCURRENCY env var. The TS helper now reads that internal env var instead of the user-facing one, leaving the public configuration surface owned by Go (and reachable in the future from config files / flags without further TS changes). - Add the new internal env var to the legacy-CLI env blacklist so a user can't bypass the Go config by setting it directly. - A new env var (not the existing MAX_THREADS) keeps HTTP request concurrency separate from the CPU-bound thread pool, per F2F. --- cliv2/cmd/cliv2/main.go | 1 + cliv2/internal/cliv2/cliv2.go | 12 ++++++ cliv2/internal/constants/constants.go | 1 + src/lib/snyk-test/common.ts | 10 +++-- test/jest/unit/lib/snyk-test/common.spec.ts | 48 ++++++++++----------- 5 files changed, 44 insertions(+), 28 deletions(-) diff --git a/cliv2/cmd/cliv2/main.go b/cliv2/cmd/cliv2/main.go index 593b653895..091d9d8d0c 100644 --- a/cliv2/cmd/cliv2/main.go +++ b/cliv2/cmd/cliv2/main.go @@ -122,6 +122,7 @@ func initApplicationConfiguration(config configuration.Configuration) { config.AddAlternativeKeys(configuration.LOG_LEVEL, []string{debug_level_flag}) config.AddAlternativeKeys(configuration.INTEGRATION_NAME, []string{integrationNameFlag}) config.AddAlternativeKeys(middleware.ConfigurationKeyRequestAttempts, []string{"snyk_max_attempts", maxNetworkRequestAttempts}) + config.AddAlternativeKeys(cliv2.ConfigKeyRequestConcurrency, []string{"snyk_request_concurrency"}) } func getFullCommandString(cmd *cobra.Command) string { diff --git a/cliv2/internal/cliv2/cliv2.go b/cliv2/internal/cliv2/cliv2.go index bac899a594..48f4dcf909 100644 --- a/cliv2/internal/cliv2/cliv2.go +++ b/cliv2/internal/cliv2/cliv2.go @@ -66,6 +66,13 @@ const ( const ( configKeyErrFile = "INTERNAL_ERR_FILE_PATH" ERROR_HAS_BEEN_DISPLAYED = "hasBeenDisplayed" + // ConfigKeyRequestConcurrency is the configuration key holding the + // resolved maximum number of concurrent in-flight Snyk dependency-test + // or dependency-monitor HTTP requests issued by the legacy CLI. The + // user-facing SNYK_REQUEST_CONCURRENCY env var feeds this key (registered + // in main.go via AddAlternativeKeys); the resolved value is forwarded to + // the legacy CLI process via constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV. + ConfigKeyRequestConcurrency = "internal_request_concurrency" ) var ( @@ -355,6 +362,7 @@ func PrepareV1EnvironmentVariables( constants.SNYK_NPM_ALL_PROXY, constants.SNYK_OPENSSL_CONF, constants.SNYK_INTERNAL_PREVIEW_FEATURES_ENABLED, + constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV, constants.DEBUG_CONST, } @@ -391,6 +399,10 @@ func fillEnvironmentFromConfig(inputAsMap map[string]string, config configuratio inputAsMap[constants.SNYK_INTERNAL_ERR_FILE] = config.GetString(configKeyErrFile) inputAsMap[constants.SNYK_TEMP_PATH] = config.GetString(configuration.TEMP_DIR_PATH) + if config.IsSet(ConfigKeyRequestConcurrency) { + inputAsMap[constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV] = config.GetString(ConfigKeyRequestConcurrency) + } + if config.GetBool(configuration.PREVIEW_FEATURES_ENABLED) { inputAsMap[constants.SNYK_INTERNAL_PREVIEW_FEATURES_ENABLED] = "1" } diff --git a/cliv2/internal/constants/constants.go b/cliv2/internal/constants/constants.go index 7b22c85e5d..ff082d0ea4 100644 --- a/cliv2/internal/constants/constants.go +++ b/cliv2/internal/constants/constants.go @@ -28,6 +28,7 @@ const DEBUG_CONST = "DEBUG" const SNYK_INTERNAL_ORGID_ENV = "SNYK_INTERNAL_ORGID" const SNYK_INTERNAL_ERR_FILE = "SNYK_ERR_FILE" const SNYK_INTERNAL_PREVIEW_FEATURES_ENABLED = "SNYK_INTERNAL_PREVIEW_FEATURES" +const SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV = "SNYK_INTERNAL_REQUEST_CONCURRENCY" const SNYK_ENDPOINT_ENV = "SNYK_API" const SNYK_ORG_ENV = "SNYK_CFG_ORG" const SNYK_OPENSSL_CONF = "OPENSSL_CONF" diff --git a/src/lib/snyk-test/common.ts b/src/lib/snyk-test/common.ts index 3116397e4f..27bfa39db2 100644 --- a/src/lib/snyk-test/common.ts +++ b/src/lib/snyk-test/common.ts @@ -111,18 +111,20 @@ export type FailOn = 'all' | 'upgradable' | 'patchable'; export const RETRY_ATTEMPTS = 3; export const RETRY_DELAY = 500; -const DEFAULT_REQUEST_CONCURRENCY = 10; +const DEFAULT_REQUEST_CONCURRENCY = 5; const MIN_REQUEST_CONCURRENCY = 1; const MAX_REQUEST_CONCURRENCY = 50; /** * Returns the maximum number of in-flight Snyk dependency-test or - * dependency-monitor HTTP requests permitted at once. Override with the - * SNYK_REQUEST_CONCURRENCY environment variable; values are clamped to + * dependency-monitor HTTP requests permitted at once. The wrapping Go CLI + * resolves the user-facing SNYK_REQUEST_CONCURRENCY env var (and any future + * config-file/flag sources) and forwards the resolved value via the internal + * SNYK_INTERNAL_REQUEST_CONCURRENCY env var read here. Values are clamped to * [MIN_REQUEST_CONCURRENCY, MAX_REQUEST_CONCURRENCY]. */ export function getRequestConcurrency(): number { - const raw = process.env.SNYK_REQUEST_CONCURRENCY; + const raw = process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY; if (!raw) { return DEFAULT_REQUEST_CONCURRENCY; } diff --git a/test/jest/unit/lib/snyk-test/common.spec.ts b/test/jest/unit/lib/snyk-test/common.spec.ts index 4df25d4c4e..07a1d6ad75 100644 --- a/test/jest/unit/lib/snyk-test/common.spec.ts +++ b/test/jest/unit/lib/snyk-test/common.spec.ts @@ -121,58 +121,58 @@ describe('getOrCreateErrorCatalogError', () => { }); describe('getRequestConcurrency', () => { - const originalValue = process.env.SNYK_REQUEST_CONCURRENCY; + const originalValue = process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY; afterEach(() => { if (originalValue === undefined) { - delete process.env.SNYK_REQUEST_CONCURRENCY; + delete process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY; } else { - process.env.SNYK_REQUEST_CONCURRENCY = originalValue; + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = originalValue; } }); - it('returns the default of 10 when SNYK_REQUEST_CONCURRENCY is unset', () => { - delete process.env.SNYK_REQUEST_CONCURRENCY; - expect(getRequestConcurrency()).toBe(10); + it('returns the default of 5 when SNYK_INTERNAL_REQUEST_CONCURRENCY is unset', () => { + delete process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY; + expect(getRequestConcurrency()).toBe(5); }); - it('returns the default of 10 when SNYK_REQUEST_CONCURRENCY is empty', () => { - process.env.SNYK_REQUEST_CONCURRENCY = ''; - expect(getRequestConcurrency()).toBe(10); + it('returns the default of 5 when SNYK_INTERNAL_REQUEST_CONCURRENCY is empty', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = ''; + expect(getRequestConcurrency()).toBe(5); }); - it('returns the parsed value when SNYK_REQUEST_CONCURRENCY is a valid integer', () => { - process.env.SNYK_REQUEST_CONCURRENCY = '15'; + it('returns the parsed value when SNYK_INTERNAL_REQUEST_CONCURRENCY is a valid integer', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = '15'; expect(getRequestConcurrency()).toBe(15); }); - it('clamps to the maximum of 50 when SNYK_REQUEST_CONCURRENCY exceeds the cap', () => { - process.env.SNYK_REQUEST_CONCURRENCY = '500'; + it('clamps to the maximum of 50 when SNYK_INTERNAL_REQUEST_CONCURRENCY exceeds the cap', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = '500'; expect(getRequestConcurrency()).toBe(50); }); - it('returns the default when SNYK_REQUEST_CONCURRENCY is below the minimum', () => { - process.env.SNYK_REQUEST_CONCURRENCY = '0'; - expect(getRequestConcurrency()).toBe(10); + it('returns the default when SNYK_INTERNAL_REQUEST_CONCURRENCY is below the minimum', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = '0'; + expect(getRequestConcurrency()).toBe(5); }); - it('returns the default when SNYK_REQUEST_CONCURRENCY is negative', () => { - process.env.SNYK_REQUEST_CONCURRENCY = '-5'; - expect(getRequestConcurrency()).toBe(10); + it('returns the default when SNYK_INTERNAL_REQUEST_CONCURRENCY is negative', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = '-5'; + expect(getRequestConcurrency()).toBe(5); }); - it('returns the default when SNYK_REQUEST_CONCURRENCY is non-numeric', () => { - process.env.SNYK_REQUEST_CONCURRENCY = 'abc'; - expect(getRequestConcurrency()).toBe(10); + it('returns the default when SNYK_INTERNAL_REQUEST_CONCURRENCY is non-numeric', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = 'abc'; + expect(getRequestConcurrency()).toBe(5); }); it('honors the minimum boundary', () => { - process.env.SNYK_REQUEST_CONCURRENCY = '1'; + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = '1'; expect(getRequestConcurrency()).toBe(1); }); it('honors the maximum boundary', () => { - process.env.SNYK_REQUEST_CONCURRENCY = '50'; + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = '50'; expect(getRequestConcurrency()).toBe(50); }); }); From b35fad090b750d616ad9e1c238ad225c305904ec Mon Sep 17 00:00:00 2001 From: Brian Gardiner Date: Wed, 13 May 2026 12:06:43 -0400 Subject: [PATCH 3/4] test: cover Go-side SNYK_REQUEST_CONCURRENCY plumbing Add unit coverage for fillEnvironmentFromConfig's handling of the new ConfigKeyRequestConcurrency: forwards a user-set value to the legacy CLI as SNYK_INTERNAL_REQUEST_CONCURRENCY, omits the internal env when unset, and strips a user-provided internal env so Go remains the source of truth. --- cliv2/internal/cliv2/cliv2_test.go | 46 ++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/cliv2/internal/cliv2/cliv2_test.go b/cliv2/internal/cliv2/cliv2_test.go index 847f66f2da..6616c75fc2 100644 --- a/cliv2/internal/cliv2/cliv2_test.go +++ b/cliv2/internal/cliv2/cliv2_test.go @@ -270,6 +270,52 @@ func Test_PrepareV1EnvironmentVariables_OnlyExplicitlySetValues(t *testing.T) { }) } +func Test_PrepareV1EnvironmentVariables_RequestConcurrency(t *testing.T) { + t.Run("forwards resolved value to internal env when alt key is set via env", func(t *testing.T) { + t.Setenv("SNYK_REQUEST_CONCURRENCY", "17") + + config := configuration.NewWithOpts(configuration.WithAutomaticEnv()) + config.AddAlternativeKeys(cliv2.ConfigKeyRequestConcurrency, []string{"snyk_request_concurrency"}) + + actual, err := cliv2.PrepareV1EnvironmentVariables([]string{}, "foo", "bar", "proxy", "cacertlocation", config, []string{}) + + assert.Nil(t, err) + assert.Contains(t, actual, constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV+"=17") + }) + + t.Run("does not set internal env when alt key is unset", func(t *testing.T) { + // guard against a stray env var leaking into the test environment + t.Setenv("SNYK_REQUEST_CONCURRENCY", "") + _ = os.Unsetenv("SNYK_REQUEST_CONCURRENCY") + + config := configuration.NewWithOpts(configuration.WithAutomaticEnv()) + config.AddAlternativeKeys(cliv2.ConfigKeyRequestConcurrency, []string{"snyk_request_concurrency"}) + + actual, err := cliv2.PrepareV1EnvironmentVariables([]string{}, "foo", "bar", "proxy", "cacertlocation", config, []string{}) + + assert.Nil(t, err) + for _, kv := range actual { + assert.NotContains(t, kv, constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV+"=") + } + }) + + t.Run("user-set internal env is stripped before Go reapplies it", func(t *testing.T) { + t.Setenv("SNYK_REQUEST_CONCURRENCY", "9") + + config := configuration.NewWithOpts(configuration.WithAutomaticEnv()) + config.AddAlternativeKeys(cliv2.ConfigKeyRequestConcurrency, []string{"snyk_request_concurrency"}) + + // Simulate a user trying to bypass Go config by setting the internal var directly. + input := []string{constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV + "=999"} + + actual, err := cliv2.PrepareV1EnvironmentVariables(input, "foo", "bar", "proxy", "cacertlocation", config, []string{}) + + assert.Nil(t, err) + assert.Contains(t, actual, constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV+"=9") + assert.NotContains(t, actual, constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV+"=999") + }) +} + func Test_PrepareV1EnvironmentVariables_Fail_DontOverrideExisting(t *testing.T) { orgid := "orgid" testapi := "https://api.snyky.io" From abe1f811623cbf7da9715b6723cf7d148da21a0f Mon Sep 17 00:00:00 2001 From: Brian Gardiner Date: Wed, 13 May 2026 20:32:32 -0400 Subject: [PATCH 4/4] fix: forward SNYK_REQUEST_CONCURRENCY via GetString (not IsSet) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under main.go's WithSupportedEnvVarPrefixes setup (the production config), GAF's IsSet does not pre-bind env vars for alternative keys — only get() does. As a result, config.IsSet(ConfigKeyRequestConcurrency) returned false even when SNYK_REQUEST_CONCURRENCY was set, so the internal env var was never forwarded to the legacy CLI process and the TS code always saw the default concurrency. Switch to GetString and check non-empty: GetString goes through GAF's get(), which binds the alt key before reading. The original unit test passed only because it used WithAutomaticEnv, which bypasses bindEnv entirely and so masked the production behavior. Update the test to construct the config the way main.go does (with WithSupportedEnvVarPrefixes), so the regression is caught next time. --- cliv2/internal/cliv2/cliv2.go | 10 ++++++++-- cliv2/internal/cliv2/cliv2_test.go | 28 ++++++++++++++++------------ 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/cliv2/internal/cliv2/cliv2.go b/cliv2/internal/cliv2/cliv2.go index 48f4dcf909..cb62d2ebce 100644 --- a/cliv2/internal/cliv2/cliv2.go +++ b/cliv2/internal/cliv2/cliv2.go @@ -399,8 +399,14 @@ func fillEnvironmentFromConfig(inputAsMap map[string]string, config configuratio inputAsMap[constants.SNYK_INTERNAL_ERR_FILE] = config.GetString(configKeyErrFile) inputAsMap[constants.SNYK_TEMP_PATH] = config.GetString(configuration.TEMP_DIR_PATH) - if config.IsSet(ConfigKeyRequestConcurrency) { - inputAsMap[constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV] = config.GetString(ConfigKeyRequestConcurrency) + // Forward the resolved request concurrency to the legacy CLI when the user + // set the value. We can't use config.IsSet here: in GAF, IsSet does not + // pre-bind env vars for alternative keys, so it returns false even when + // the SNYK_REQUEST_CONCURRENCY env var is set under WithSupportedEnvVarPrefixes + // (the production setup). GetString goes through GAF's get(), which binds + // the alt key before reading, so it returns the resolved value correctly. + if v := config.GetString(ConfigKeyRequestConcurrency); v != "" { + inputAsMap[constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV] = v } if config.GetBool(configuration.PREVIEW_FEATURES_ENABLED) { diff --git a/cliv2/internal/cliv2/cliv2_test.go b/cliv2/internal/cliv2/cliv2_test.go index 6616c75fc2..54da22d7ae 100644 --- a/cliv2/internal/cliv2/cliv2_test.go +++ b/cliv2/internal/cliv2/cliv2_test.go @@ -271,13 +271,23 @@ func Test_PrepareV1EnvironmentVariables_OnlyExplicitlySetValues(t *testing.T) { } func Test_PrepareV1EnvironmentVariables_RequestConcurrency(t *testing.T) { + // Mirror main.go's production configuration setup. Crucially, this uses + // WithSupportedEnvVarPrefixes (NOT WithAutomaticEnv): under that setup, + // GAF's IsSet does not pre-bind env vars for alternative keys, so any + // implementation that gates forwarding on IsSet would fail to forward + // the value. This test catches that regression. + newConfig := func() configuration.Configuration { + c := configuration.NewWithOpts( + configuration.WithSupportedEnvVarPrefixes("snyk_", "internal_", "test_"), + ) + c.AddAlternativeKeys(cliv2.ConfigKeyRequestConcurrency, []string{"snyk_request_concurrency"}) + return c + } + t.Run("forwards resolved value to internal env when alt key is set via env", func(t *testing.T) { t.Setenv("SNYK_REQUEST_CONCURRENCY", "17") - config := configuration.NewWithOpts(configuration.WithAutomaticEnv()) - config.AddAlternativeKeys(cliv2.ConfigKeyRequestConcurrency, []string{"snyk_request_concurrency"}) - - actual, err := cliv2.PrepareV1EnvironmentVariables([]string{}, "foo", "bar", "proxy", "cacertlocation", config, []string{}) + actual, err := cliv2.PrepareV1EnvironmentVariables([]string{}, "foo", "bar", "proxy", "cacertlocation", newConfig(), []string{}) assert.Nil(t, err) assert.Contains(t, actual, constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV+"=17") @@ -288,10 +298,7 @@ func Test_PrepareV1EnvironmentVariables_RequestConcurrency(t *testing.T) { t.Setenv("SNYK_REQUEST_CONCURRENCY", "") _ = os.Unsetenv("SNYK_REQUEST_CONCURRENCY") - config := configuration.NewWithOpts(configuration.WithAutomaticEnv()) - config.AddAlternativeKeys(cliv2.ConfigKeyRequestConcurrency, []string{"snyk_request_concurrency"}) - - actual, err := cliv2.PrepareV1EnvironmentVariables([]string{}, "foo", "bar", "proxy", "cacertlocation", config, []string{}) + actual, err := cliv2.PrepareV1EnvironmentVariables([]string{}, "foo", "bar", "proxy", "cacertlocation", newConfig(), []string{}) assert.Nil(t, err) for _, kv := range actual { @@ -302,13 +309,10 @@ func Test_PrepareV1EnvironmentVariables_RequestConcurrency(t *testing.T) { t.Run("user-set internal env is stripped before Go reapplies it", func(t *testing.T) { t.Setenv("SNYK_REQUEST_CONCURRENCY", "9") - config := configuration.NewWithOpts(configuration.WithAutomaticEnv()) - config.AddAlternativeKeys(cliv2.ConfigKeyRequestConcurrency, []string{"snyk_request_concurrency"}) - // Simulate a user trying to bypass Go config by setting the internal var directly. input := []string{constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV + "=999"} - actual, err := cliv2.PrepareV1EnvironmentVariables(input, "foo", "bar", "proxy", "cacertlocation", config, []string{}) + actual, err := cliv2.PrepareV1EnvironmentVariables(input, "foo", "bar", "proxy", "cacertlocation", newConfig(), []string{}) assert.Nil(t, err) assert.Contains(t, actual, constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV+"=9")