-
Notifications
You must be signed in to change notification settings - Fork 169
fix: initialize global HTTP dispatcher for proxy support #1070
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
MaUhlik-cen56998
wants to merge
1
commit into
actions:main
Choose a base branch
from
MaUhlik-cen56998:fix-proxy-issue
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+318
−9
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,209 @@ | ||
| import {expect, test, jest, beforeEach, afterEach} from '@jest/globals' | ||
| import {initializeProxySupport} from '../src/proxy' | ||
| import * as core from '@actions/core' | ||
| import { | ||
| EnvHttpProxyAgent, | ||
| setGlobalDispatcher, | ||
| getGlobalDispatcher, | ||
| Dispatcher | ||
| } from 'undici' | ||
|
|
||
| // Mock @actions/core | ||
| jest.mock('@actions/core') | ||
|
|
||
| describe('proxy support', () => { | ||
| let originalEnv: NodeJS.ProcessEnv | ||
| let mockDebug: jest.MockedFunction<typeof core.debug> | ||
| let mockInfo: jest.MockedFunction<typeof core.info> | ||
| let mockWarning: jest.MockedFunction<typeof core.warning> | ||
| let originalDispatcher: Dispatcher | ||
|
|
||
| beforeEach(() => { | ||
| // Save original environment | ||
| originalEnv = {...process.env} | ||
| originalDispatcher = getGlobalDispatcher() | ||
|
|
||
| delete process.env.HTTP_PROXY | ||
| delete process.env.HTTPS_PROXY | ||
| delete process.env.http_proxy | ||
| delete process.env.https_proxy | ||
|
|
||
| // Setup mocks | ||
| mockDebug = jest.mocked(core.debug) | ||
| mockInfo = jest.mocked(core.info) | ||
| mockWarning = jest.mocked(core.warning) | ||
|
|
||
| mockDebug.mockClear() | ||
| mockInfo.mockClear() | ||
| mockWarning.mockClear() | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| // Restore original environment | ||
| process.env = originalEnv | ||
| setGlobalDispatcher(originalDispatcher) | ||
| }) | ||
|
|
||
| test('does nothing when no proxy is configured', () => { | ||
| initializeProxySupport() | ||
|
|
||
| expect(mockDebug).toHaveBeenCalledWith('No proxy configuration detected') | ||
| expect(mockInfo).not.toHaveBeenCalled() | ||
| expect(mockWarning).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| test('configures proxy from HTTPS_PROXY environment variable', () => { | ||
| process.env.HTTPS_PROXY = 'http://proxy.company.com:8080' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| expect(mockDebug).toHaveBeenCalledWith( | ||
| 'Proxy configured: proxy.company.com:8080' | ||
| ) | ||
| expect(mockWarning).not.toHaveBeenCalled() | ||
| expect(mockDebug).not.toHaveBeenCalledWith( | ||
| 'No proxy configuration detected' | ||
| ) | ||
|
|
||
| const dispatcher = getGlobalDispatcher() | ||
| expect(dispatcher).toBeInstanceOf(EnvHttpProxyAgent) | ||
| }) | ||
|
|
||
| test('configures proxy from https_proxy environment variable (lowercase)', () => { | ||
| process.env.https_proxy = 'http://proxy.example.com:3128' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| expect(mockDebug).toHaveBeenCalledWith( | ||
| 'Proxy configured: proxy.example.com:3128' | ||
| ) | ||
| const dispatcher = getGlobalDispatcher() | ||
| expect(dispatcher).toBeInstanceOf(EnvHttpProxyAgent) | ||
| }) | ||
|
|
||
| test('configures proxy from HTTP_PROXY environment variable', () => { | ||
| process.env.HTTP_PROXY = 'http://proxy.example.com:8888' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| expect(mockDebug).toHaveBeenCalledWith( | ||
| 'Proxy configured: proxy.example.com:8888' | ||
| ) | ||
| }) | ||
|
|
||
| test('configures proxy from http_proxy environment variable (lowercase)', () => { | ||
| process.env.http_proxy = 'http://proxy.test.com:9090' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| expect(mockDebug).toHaveBeenCalledWith( | ||
| 'Proxy configured: proxy.test.com:9090' | ||
| ) | ||
| }) | ||
|
|
||
| test('prioritizes uppercase over lowercase', () => { | ||
| process.env.HTTPS_PROXY = 'http://uppercase.com:8080' | ||
| process.env.https_proxy = 'http://lowercase.com:8080' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| expect(mockDebug).toHaveBeenCalledWith( | ||
| 'Proxy configured: uppercase.com:8080' | ||
| ) | ||
| }) | ||
|
|
||
| test('handles proxy with authentication credentials', () => { | ||
| process.env.HTTPS_PROXY = 'http://user:pass@proxy.secure.com:8080' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| // Should log proxy without showing credentials | ||
| expect(mockDebug).toHaveBeenCalledWith( | ||
| 'Proxy configured: proxy.secure.com:8080' | ||
| ) | ||
| expect(mockWarning).not.toHaveBeenCalled() | ||
|
|
||
| const dispatcher = getGlobalDispatcher() | ||
| expect(dispatcher).toBeInstanceOf(EnvHttpProxyAgent) | ||
| }) | ||
|
|
||
| test('handles proxy with URL-encoded credentials', () => { | ||
| process.env.HTTPS_PROXY = | ||
| 'http://user%40domain:p%40ss%3Aword@proxy.com:8080' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| expect(mockDebug).toHaveBeenCalledWith('Proxy configured: proxy.com:8080') | ||
| expect(mockWarning).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| test('handles proxy with HTTPS scheme', () => { | ||
| process.env.HTTPS_PROXY = 'https://secure-proxy.com:443' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| expect(mockDebug).toHaveBeenCalledWith( | ||
| 'Proxy configured: secure-proxy.com:443' | ||
| ) | ||
| }) | ||
|
|
||
| test('uses default port 443 for https proxy without explicit port', () => { | ||
| process.env.HTTPS_PROXY = 'https://proxy.com' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| expect(mockDebug).toHaveBeenCalledWith('Proxy configured: proxy.com:443') | ||
| }) | ||
|
|
||
| test('uses default port 80 for http proxy without explicit port', () => { | ||
| process.env.HTTP_PROXY = 'http://proxy.com' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| expect(mockDebug).toHaveBeenCalledWith('Proxy configured: proxy.com:80') | ||
| }) | ||
|
|
||
| test('handles invalid proxy URL gracefully', () => { | ||
| process.env.HTTPS_PROXY = 'not-a-valid-url' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| expect(mockWarning).toHaveBeenCalled() | ||
| const warningCall = mockWarning.mock.calls[0][0] as string | ||
| expect(warningCall).toContain('Failed to configure proxy') | ||
| expect(warningCall).toContain('not-a-valid-url') | ||
| }) | ||
|
|
||
| test('handles proxy URL with only hostname (no scheme)', () => { | ||
| process.env.HTTPS_PROXY = 'proxy.company.com:8080' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| // Should fail gracefully as URL requires a scheme | ||
| expect(mockWarning).toHaveBeenCalled() | ||
| const warningCall = mockWarning.mock.calls[0][0] as string | ||
| expect(warningCall).toContain('Failed to configure proxy') | ||
| }) | ||
|
|
||
| test('redacts credentials in scheme-less malformed proxy URL logs', () => { | ||
| process.env.HTTPS_PROXY = 'user:super-secret@proxy.company.com:8080' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| expect(mockWarning).toHaveBeenCalled() | ||
| const warningCall = mockWarning.mock.calls[0][0] as string | ||
| expect(warningCall).toContain('Failed to configure proxy') | ||
| expect(warningCall).toContain('[REDACTED]@proxy.company.com:8080') | ||
| expect(warningCall).not.toContain('user:super-secret') | ||
| expect(warningCall).not.toContain('super-secret') | ||
| }) | ||
|
|
||
| test('handles empty proxy URL', () => { | ||
| process.env.HTTPS_PROXY = '' | ||
|
|
||
| initializeProxySupport() | ||
|
|
||
| // Empty string is falsy, should detect as no proxy | ||
| expect(mockDebug).toHaveBeenCalledWith('No proxy configuration detected') | ||
| }) | ||
| }) | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import {EnvHttpProxyAgent, setGlobalDispatcher} from 'undici' | ||
| import * as core from '@actions/core' | ||
|
|
||
| function sanitizeProxyUrlForLogging(proxyUrl: string): string { | ||
| try { | ||
| const url = new URL(proxyUrl) | ||
| if ( | ||
| (url.protocol !== 'http:' && url.protocol !== 'https:') || | ||
| url.hostname === '' | ||
| ) { | ||
| throw new Error('Invalid proxy URL format') | ||
| } | ||
|
|
||
| const port = url.port || (url.protocol === 'https:' ? '443' : '80') | ||
| return `${url.hostname}:${port}` | ||
| } catch { | ||
| // Redact anything before the last '@' to also cover scheme-less input | ||
| // like "user:pass@proxy:8080". | ||
| const atIndex = proxyUrl.lastIndexOf('@') | ||
| if (atIndex === -1) { | ||
| return proxyUrl | ||
| } | ||
|
|
||
| const hostPart = proxyUrl.slice(atIndex + 1) | ||
| const schemeMatch = proxyUrl.match(/^[a-zA-Z][a-zA-Z\d+.-]*:\/\//) | ||
|
|
||
| if (schemeMatch) { | ||
| return `${schemeMatch[0]}[REDACTED]@${hostPart}` | ||
| } | ||
|
|
||
| return `[REDACTED]@${hostPart}` | ||
| } | ||
| } | ||
|
|
||
| function getErrorMessage(error: unknown): string { | ||
| if (error instanceof Error) { | ||
| return error.message | ||
| } | ||
| return String(error) | ||
| } | ||
|
|
||
| /** | ||
| * Initializes proxy support for native fetch() API in Node.js 20+. | ||
| * Uses undici's EnvHttpProxyAgent which automatically reads proxy configuration | ||
| * from environment variables (HTTP_PROXY, HTTPS_PROXY, NO_PROXY, etc.) | ||
| * and handles credential extraction from proxy URLs. | ||
| * | ||
| * This must be called early in the application lifecycle before any fetch() calls. | ||
| */ | ||
| export function initializeProxySupport(): void { | ||
| const proxyUrl = | ||
| process.env.HTTPS_PROXY || | ||
| process.env.https_proxy || | ||
| process.env.HTTP_PROXY || | ||
| process.env.http_proxy | ||
MaUhlik-cen56998 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (!proxyUrl) { | ||
| core.debug('No proxy configuration detected') | ||
| return | ||
| } | ||
|
|
||
| try { | ||
| const agent = new EnvHttpProxyAgent() | ||
| setGlobalDispatcher(agent) | ||
|
|
||
| // Log proxy host without credentials | ||
| core.debug(`Proxy configured: ${sanitizeProxyUrlForLogging(proxyUrl)}`) | ||
| } catch (error: unknown) { | ||
| const sanitizedProxyUrl = sanitizeProxyUrlForLogging(proxyUrl) | ||
| core.warning( | ||
| `Failed to configure proxy from ${sanitizedProxyUrl}: ${getErrorMessage(error)}` | ||
| ) | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.