From e693e659f234f1c09f9dbeb905cbabbf9cb5b8ca Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 29 Jul 2026 21:02:50 +0100 Subject: [PATCH 1/4] fix(legacy): use the detected CA bundle for the login token request The authorization code exchange in auth:browser-login built its own Guzzle client with `verify => true`. Guzzle then enables peer verification without setting CURLOPT_CAINFO, so curl falls back to its default CA source. Every other request in the CLI passes an explicit path from Composer\CaBundle, which honors SSL_CERT_FILE and SSL_CERT_DIR. Behind a TLS-inspecting proxy that made login fail at the last step with "cURL error 60: SSL certificate problem", even though the rest of the CLI worked with the corporate root CA in SSL_CERT_FILE. On Windows the fallback is always wrong: the wrapper runs PHP with -n and -d openssl.cafile pointing at the bundled Mozilla CA list, and PHP's curl extension takes its default CAINFO from that setting, so this request could only ever trust the bundled list. Use Api::getExternalHttpClient() instead, which applies the detected CA bundle, the configured proxy, the user agent and the api.skip_ssl option. It logs the request line and status at -vvv, never message bodies, so tokens stay out of the output. Reported in https://github.com/upsun/cli/issues/110 Co-Authored-By: Claude Opus 5 (1M context) --- .../src/Command/Auth/BrowserLoginCommand.php | 5 +- .../Command/Auth/BrowserLoginCommandTest.php | 72 +++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 legacy/tests/Command/Auth/BrowserLoginCommandTest.php diff --git a/legacy/src/Command/Auth/BrowserLoginCommand.php b/legacy/src/Command/Auth/BrowserLoginCommand.php index 5c667c062..969dec0b9 100644 --- a/legacy/src/Command/Auth/BrowserLoginCommand.php +++ b/legacy/src/Command/Auth/BrowserLoginCommand.php @@ -9,7 +9,6 @@ use Platformsh\Cli\Service\Api; use Platformsh\Cli\Service\Config; use Platformsh\Cli\Service\QuestionHelper; -use GuzzleHttp\Client; use GuzzleHttp\Exception\BadResponseException; use GuzzleHttp\Psr7\Request; use GuzzleHttp\Utils; @@ -313,7 +312,9 @@ private function createDocumentRoot(string $dir): void */ private function getAccessToken(string $authCode, string $codeVerifier, string $redirectUri): array { - $client = new Client(['verify' => !$this->config->getBool('api.skip_ssl')]); + // Use the shared HTTP client, so that the request uses the detected CA + // bundle and any configured proxy. + $client = $this->api->getExternalHttpClient(); $request = new Request('POST', $this->config->getStr('api.oauth2_token_url'), body: http_build_query([ 'grant_type' => 'authorization_code', 'code' => $authCode, diff --git a/legacy/tests/Command/Auth/BrowserLoginCommandTest.php b/legacy/tests/Command/Auth/BrowserLoginCommandTest.php new file mode 100644 index 000000000..ae0c595fb --- /dev/null +++ b/legacy/tests/Command/Auth/BrowserLoginCommandTest.php @@ -0,0 +1,72 @@ +createMock(ClientInterface::class); + $httpClient->expects($this->once()) + ->method('send') + ->willReturnCallback(function (RequestInterface $request) use (&$sentRequest): Response { + $sentRequest = $request; + return new Response(200, [], '{"access_token": "test-token", "token_type": "bearer"}'); + }); + + $api = $this->createMock(Api::class); + $api->expects($this->once()) + ->method('getExternalHttpClient') + ->willReturn($httpClient); + + $values = [ + 'api.oauth2_token_url' => $tokenUrl, + 'api.oauth2_client_id' => 'test-client-id', + ]; + $config = $this->createMock(Config::class); + $config->method('getStr')->willReturnCallback(fn(string $key): string => $values[$key] ?? ''); + $config->method('get')->willReturnCallback(fn(string $key): string => $values[$key] ?? ''); + + $command = new BrowserLoginCommand( + $api, + $config, + $this->createMock(Io::class), + $this->createMock(Login::class), + $this->createMock(QuestionHelper::class), + $this->createMock(Url::class), + ); + + $method = new \ReflectionMethod($command, 'getAccessToken'); + $token = $method->invoke($command, 'test-code', 'test-verifier', 'http://127.0.0.1:5000'); + + $this->assertSame('test-token', $token['access_token']); + $this->assertInstanceOf(RequestInterface::class, $sentRequest); + $this->assertSame($tokenUrl, (string) $sentRequest->getUri()); + } +} From 8576cf2e340cd9e2a5dc2027c8e46e223e9ef5ca Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 29 Jul 2026 21:02:50 +0100 Subject: [PATCH 2/4] fix(legacy): do not let the login server parse an unsuitable php.ini The local OAuth listener server is started with the same PHP binary as the CLI, but without -n, so it parses any php.ini it can find even when the CLI itself was started without one. Under the Go wrapper that means the static PHP binary loads a system php.ini, which produced warnings like 'Module "curl" is already loaded' for one user, and could stop the server from starting at all with a php.ini that enables extensions this build cannot load. Pass -n to the server when this process parsed no php.ini either. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/Command/Auth/BrowserLoginCommand.php | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/legacy/src/Command/Auth/BrowserLoginCommand.php b/legacy/src/Command/Auth/BrowserLoginCommand.php index 969dec0b9..1e5b40047 100644 --- a/legacy/src/Command/Auth/BrowserLoginCommand.php +++ b/legacy/src/Command/Auth/BrowserLoginCommand.php @@ -140,14 +140,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int chmod($responseFile, 0o600); // Start the local server. - $process = new Process([ - (new PhpExecutableFinder())->find() ?: PHP_BINARY, - '-dvariables_order=egps', - '-S', - $localAddress, - '-t', - $listenerDir, - ]); + $phpCommand = [(new PhpExecutableFinder())->find() ?: PHP_BINARY]; + if (!php_ini_loaded_file() && !php_ini_scanned_files()) { + // This process parsed no php.ini file, so tell the server not to + // parse one either. Otherwise it may load a php.ini which does not + // suit this PHP build, for example one that enables extensions + // which are already built in. + $phpCommand[] = '-n'; + } + $phpCommand[] = '-dvariables_order=egps'; + $process = new Process(array_merge($phpCommand, ['-S', $localAddress, '-t', $listenerDir])); $codeVerifier = $this->generateCodeVerifier(); $process->setEnv([ 'CLI_OAUTH_APP_NAME' => $this->config->getStr('application.name'), From f2083751137ff9fb0011d283ed6c9ef8ea538376 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 29 Jul 2026 21:58:08 +0100 Subject: [PATCH 3/4] fix(legacy): honor the no_proxy environment variable Guzzle reads http_proxy, https_proxy and no_proxy itself, but only when the proxy request option is absent. The CLI always sets that option from Config::getProxies(), so the no-proxy list was silently dropped for every API request. This also keeps the login token request behaving as it did before it started using the shared HTTP client, which until then was left to Guzzle's own environment handling. Both cases of the variable name are accepted, since the effect is only ever to send a request directly instead of through a proxy. Co-Authored-By: Claude Opus 5 (1M context) --- legacy/src/Service/Api.php | 12 ++++++++++-- legacy/src/Service/Config.php | 18 ++++++++++++++++++ legacy/tests/ConfigTest.php | 21 +++++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/legacy/src/Service/Api.php b/legacy/src/Service/Api.php index d96c106a3..1bd15d9a7 100644 --- a/legacy/src/Service/Api.php +++ b/legacy/src/Service/Api.php @@ -485,11 +485,11 @@ private function tokenFromSession(SessionInterface $session): ?AccessToken /** * Returns proxy config in the format expected by Guzzle. * - * @return string[] + * @return array */ private function guzzleProxyConfig(): array { - return array_map(function ($proxyUrl) { + $config = array_map(function ($proxyUrl) { // If Guzzle is going to use PHP's built-in HTTP streams, // rather than curl, then transform the proxy scheme. if (!\extension_loaded('curl') && \ini_get('allow_url_fopen')) { @@ -497,6 +497,14 @@ private function guzzleProxyConfig(): array } return $proxyUrl; }, $this->config->getProxies()); + + // Guzzle only reads the no_proxy environment variable itself when no + // proxy has been configured explicitly, so pass the hosts on. + if ($noProxy = $this->config->getNoProxy()) { + $config['no'] = $noProxy; + } + + return $config; } /** diff --git a/legacy/src/Service/Config.php b/legacy/src/Service/Config.php index dc24e54a2..fb33936ba 100644 --- a/legacy/src/Service/Config.php +++ b/legacy/src/Service/Config.php @@ -601,6 +601,24 @@ public function getProxies(): array return $proxies; } + /** + * Finds the hosts which should not be proxied, from the no_proxy environment variable. + * + * @return string[] + */ + public function getNoProxy(): array + { + $value = \getenv('no_proxy'); + if ($value === false) { + $value = \getenv('NO_PROXY'); + } + if ($value === false) { + return []; + } + $hosts = \array_map('trim', \explode(',', $value)); + return \array_values(\array_filter($hosts, fn(string $host): bool => $host !== '')); + } + /** * Returns an array of context options for HTTP/HTTPS streams. * diff --git a/legacy/tests/ConfigTest.php b/legacy/tests/ConfigTest.php index 8427112dd..df32194e4 100644 --- a/legacy/tests/ConfigTest.php +++ b/legacy/tests/ConfigTest.php @@ -125,4 +125,25 @@ public function testGetWritableUserDir(): void $home = $config->getHomeDirectory(); $this->assertEquals($home . DIRECTORY_SEPARATOR . 'mock-cli-user-config', $config->getWritableUserDir()); } + + /** + * Test finding the hosts which should not be proxied. + */ + public function testGetNoProxy(): void + { + $config = new Config([], $this->configFile); + try { + $this->assertEquals([], $config->getNoProxy()); + + putenv('no_proxy=example.com, .example.net,'); + $this->assertEquals(['example.com', '.example.net'], $config->getNoProxy()); + putenv('no_proxy'); + + putenv('NO_PROXY=example.org'); + $this->assertEquals(['example.org'], $config->getNoProxy()); + } finally { + putenv('no_proxy'); + putenv('NO_PROXY'); + } + } } From 184f2ddae82c056511df63a751c1019b581376ec Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 30 Jul 2026 00:44:27 +0100 Subject: [PATCH 4/4] test(legacy): make the new tests independent of the environment The no_proxy test asserted an empty list without first clearing the variables, so it failed where they are already set, and it cleared them afterwards instead of restoring them. It now saves and restores both. The login test also records the request options, so that it covers the client credentials still being sent. Written by Claude Code. --- legacy/tests/Command/Auth/BrowserLoginCommandTest.php | 5 ++++- legacy/tests/ConfigTest.php | 8 ++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/legacy/tests/Command/Auth/BrowserLoginCommandTest.php b/legacy/tests/Command/Auth/BrowserLoginCommandTest.php index ae0c595fb..d59627519 100644 --- a/legacy/tests/Command/Auth/BrowserLoginCommandTest.php +++ b/legacy/tests/Command/Auth/BrowserLoginCommandTest.php @@ -32,11 +32,13 @@ public function testAccessTokenRequestUsesSharedHttpClient(): void $tokenUrl = 'https://auth.example.com/oauth2/token'; $sentRequest = null; + $sentOptions = []; $httpClient = $this->createMock(ClientInterface::class); $httpClient->expects($this->once()) ->method('send') - ->willReturnCallback(function (RequestInterface $request) use (&$sentRequest): Response { + ->willReturnCallback(function (RequestInterface $request, array $options = []) use (&$sentRequest, &$sentOptions): Response { $sentRequest = $request; + $sentOptions = $options; return new Response(200, [], '{"access_token": "test-token", "token_type": "bearer"}'); }); @@ -68,5 +70,6 @@ public function testAccessTokenRequestUsesSharedHttpClient(): void $this->assertSame('test-token', $token['access_token']); $this->assertInstanceOf(RequestInterface::class, $sentRequest); $this->assertSame($tokenUrl, (string) $sentRequest->getUri()); + $this->assertSame(['test-client-id', ''], $sentOptions['auth'] ?? null); } } diff --git a/legacy/tests/ConfigTest.php b/legacy/tests/ConfigTest.php index df32194e4..5720cf441 100644 --- a/legacy/tests/ConfigTest.php +++ b/legacy/tests/ConfigTest.php @@ -132,7 +132,10 @@ public function testGetWritableUserDir(): void public function testGetNoProxy(): void { $config = new Config([], $this->configFile); + $original = ['no_proxy' => getenv('no_proxy'), 'NO_PROXY' => getenv('NO_PROXY')]; try { + putenv('no_proxy'); + putenv('NO_PROXY'); $this->assertEquals([], $config->getNoProxy()); putenv('no_proxy=example.com, .example.net,'); @@ -142,8 +145,9 @@ public function testGetNoProxy(): void putenv('NO_PROXY=example.org'); $this->assertEquals(['example.org'], $config->getNoProxy()); } finally { - putenv('no_proxy'); - putenv('NO_PROXY'); + foreach ($original as $name => $value) { + putenv($value === false ? $name : $name . '=' . $value); + } } } }