diff --git a/.github/workflows/test-python.yml b/.github/workflows/test-python.yml index bfc005bf..c2810722 100644 --- a/.github/workflows/test-python.yml +++ b/.github/workflows/test-python.yml @@ -72,13 +72,16 @@ jobs: - name: Checkout uses: actions/checkout@v6 - # The refs/heads/ form serves the same manifest as the default mirror but - # deliberately does not match {owner}/{repo}/{branch}, so this exercises - # the direct-URL manifest fetch that the default coordinates skip. + # The refs/heads/ form serves the same manifest as the default mirror and + # exercises the refs/heads/{branch} -> {branch} parsing on the API path. + # check-latest forces a manifest fetch even though 3.12 is preinstalled, + # so the job actually contacts the mirror instead of short-circuiting on + # the tool cache. - name: setup-python with explicit mirror uses: ./ with: python-version: 3.12 + check-latest: true mirror: https://raw.githubusercontent.com/actions/python-versions/refs/heads/main - name: Run simple code diff --git a/__tests__/install-python-mirror.test.ts b/__tests__/install-python-mirror.test.ts index 6a0854f9..34d88e9d 100644 --- a/__tests__/install-python-mirror.test.ts +++ b/__tests__/install-python-mirror.test.ts @@ -86,6 +86,7 @@ const { getManifestFromRepo, getManifestFromURL, resolveRepoCoords, + isMirrorCustomized, installCpythonFromRelease } = await import('../src/install-python.js'); @@ -152,6 +153,55 @@ describe('getManifestUrl', () => { // find-python.ts calls this while building the "version not found" message. expect(() => getManifestUrl()).toThrow(/Invalid 'mirror' URL/); }); + + it('treats an invalid mirror as fatal on the auth path too', async () => { + // getManifestUrl() throws on a bad mirror; the auth resolution must agree + // rather than swallow the error and quietly skip the mirror-token branch. + setInputs({'mirror-token': 'MTOK', mirror: 'not a url'}); + (tc.downloadTool as jest.Mock).mockResolvedValue('/tmp/py.tgz'); + (tc.extractTar as jest.Mock).mockResolvedValue('/tmp/extracted'); + + const release = { + version: '3.12.0', + stable: true, + files: [ + { + filename: 'python-3.12.0-linux-x64.tar.gz', + platform: 'linux', + arch: 'x64', + download_url: 'https://cdn.example/py.tar.gz' + } + ] + } as any; + + await expect(installCpythonFromRelease(release)).rejects.toThrow( + /Invalid 'mirror' URL/ + ); + }); +}); + +describe('isMirrorCustomized', () => { + it('is false when the mirror input is empty', () => { + expect(isMirrorCustomized()).toBe(false); + }); + + it('is false when the mirror input equals the default', () => { + // action.yml gives `mirror` this exact default, so getInput() returns it on + // every run where the user did not set one. The PyPy/GraalPy warning must + // not fire in that case. + setInputs({mirror: DEFAULT_MIRROR}); + expect(isMirrorCustomized()).toBe(false); + }); + + it('is false when the mirror input is the default with trailing slashes', () => { + setInputs({mirror: `${DEFAULT_MIRROR}///`}); + expect(isMirrorCustomized()).toBe(false); + }); + + it('is true when the mirror input is a custom URL', () => { + setInputs({mirror: 'https://mirror.example/py'}); + expect(isMirrorCustomized()).toBe(true); + }); }); describe('resolveRepoCoords', () => { @@ -166,6 +216,22 @@ describe('resolveRepoCoords', () => { ); }); + it('parses the refs/heads/{branch} form to the bare branch without warning', () => { + setInputs({ + mirror: + 'https://raw.githubusercontent.com/actions/python-versions/refs/heads/main' + }); + + expect(resolveRepoCoords()).toEqual({ + owner: 'actions', + repo: 'python-versions', + branch: 'main' + }); + // refs/heads/main is a valid single branch, so it must route through the + // API path and never hit the slash-branch warning. + expect(core.warning).not.toHaveBeenCalled(); + }); + it('does not warn for a non-GitHub mirror', () => { setInputs({mirror: 'https://mirror.example/py'}); @@ -223,6 +289,25 @@ describe('getManifestFromRepo mirror resolution', () => { ); }); + it('resolves the refs/heads/{branch} form to the bare branch for the API', async () => { + setInputs({ + token: 'TKN', + mirror: 'https://raw.githubusercontent.com/foo/bar/refs/heads/main' + }); + (tc.getManifestFromRepo as jest.Mock).mockResolvedValue(mockManifest); + + await getManifestFromRepo(); + + // The GitHub tree API takes a bare branch, so the refs/heads/ prefix must + // be stripped rather than passed through as part of the branch name. + expect(tc.getManifestFromRepo).toHaveBeenCalledWith( + 'foo', + 'bar', + 'token TKN', + 'main' + ); + }); + it('returns null for a non-GitHub mirror so the caller uses the raw URL', () => { setInputs({mirror: 'https://mirror.example/py'}); expect(resolveRepoCoords()).toBeNull(); @@ -296,9 +381,13 @@ describe('getManifestFromURL mirror resolution', () => { }); it('sends token as a prefixed header for a GitHub-hosted raw manifest', async () => { + // A slash-branch URL genuinely falls to the direct-URL path (it does not + // match the {owner}/{repo}/{branch} shape). raw.githubusercontent.com is a + // GitHub host, so the token is still attached here — contradicting any + // claim that the fallback fetch is anonymous. setInputs({ token: 'TKN', - mirror: 'https://raw.githubusercontent.com/foo/bar/refs/heads/main' + mirror: 'https://raw.githubusercontent.com/foo/bar/feature/riscv' }); const getJson = jest.fn(async () => ({result: mockManifest})); (httpm.HttpClient as jest.Mock).mockImplementation(() => ({getJson})); @@ -306,7 +395,7 @@ describe('getManifestFromURL mirror resolution', () => { await getManifestFromURL(); expect(getJson).toHaveBeenCalledWith( - 'https://raw.githubusercontent.com/foo/bar/refs/heads/main/versions-manifest.json', + 'https://raw.githubusercontent.com/foo/bar/feature/riscv/versions-manifest.json', {authorization: 'token TKN'} ); }); @@ -426,6 +515,41 @@ describe('installCpythonFromRelease auth gating', () => { ).resolves.toBe('Basic dXNlcjpwYXNz'); }); + it('withholds mirror-token from a same-host download URL on a different scheme', async () => { + setInputs({ + 'mirror-token': 'Bearer MTOK', + mirror: 'https://cdn.example' + }); + // The mirror is https, but the manifest points a download_url at http on + // the same host. Matching on origin (scheme + host + port) rather than host + // alone keeps the token from going out in cleartext. + await expect( + downloadAuthFor('http://cdn.example/py.tar.gz') + ).resolves.toBeUndefined(); + }); + + it('withholds mirror-token from a same-host download URL on a different port', async () => { + setInputs({ + 'mirror-token': 'Bearer MTOK', + mirror: 'https://cdn.example' + }); + // Different port is a different origin, so the nominated credential must + // not follow. + await expect( + downloadAuthFor('https://cdn.example:8443/py.tar.gz') + ).resolves.toBeUndefined(); + }); + + it('sends mirror-token to a same-origin download URL on an explicit port', async () => { + setInputs({ + 'mirror-token': 'Bearer MTOK', + mirror: 'https://cdn.example:8443' + }); + await expect( + downloadAuthFor('https://cdn.example:8443/py.tar.gz') + ).resolves.toBe('Bearer MTOK'); + }); + it('withholds mirror-token from an incidental GitHub host and uses token there', async () => { setInputs({ token: 'TKN', diff --git a/dist/setup/index.js b/dist/setup/index.js index 276546e1..2983a65e 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -98697,8 +98697,11 @@ const DEFAULT_REPO_OWNER = 'actions'; const DEFAULT_REPO_NAME = 'python-versions'; const DEFAULT_REPO_BRANCH = 'main'; const DEFAULT_MIRROR = `https://raw.githubusercontent.com/${DEFAULT_REPO_OWNER}/${DEFAULT_REPO_NAME}/${DEFAULT_REPO_BRANCH}`; -// Matches https://raw.githubusercontent.com/{owner}/{repo}/{branch} -const REPO_COORDS_RE = /^https:\/\/raw\.githubusercontent\.com\/([^/]+)\/([^/]+)\/([^/]+)\/?$/; +// Matches https://raw.githubusercontent.com/{owner}/{repo}/{branch} and the +// equivalent https://raw.githubusercontent.com/{owner}/{repo}/refs/heads/{branch} +// form, capturing the bare branch in both. The GitHub tree API wants the bare +// branch, so the optional refs/heads/ prefix is consumed, not captured. +const REPO_COORDS_RE = /^https:\/\/raw\.githubusercontent\.com\/([^/]+)\/([^/]+)\/(?:refs\/heads\/)?([^/]+)\/?$/; function getToken() { return getInput('token'); } @@ -98731,13 +98734,26 @@ function getMirror() { function getManifestUrl() { return `${getMirror()}/versions-manifest.json`; } -function getMirrorHost() { - try { - return new URL(getMirror()).host; - } - catch { - return undefined; - } +// Whether the user set `mirror` to something other than the built-in default. +// action.yml gives `mirror` a default, so core.getInput('mirror') is never +// empty; callers that want "did the user opt into a custom mirror" must compare +// against DEFAULT_MIRROR rather than test for a falsy input. Normalizes the +// same way getMirror() does but never throws, so a warning path can call it +// even when the mirror is malformed. +function isMirrorCustomized() { + const input = getInput('mirror'); + if (!input) + return false; + return input.trim().replace(/\/+$/, '') !== DEFAULT_MIRROR; +} +// Origin (scheme + host + port) of the mirror, so `mirror-token` is matched +// against the exact origin the user nominated. Comparing origin rather than +// host alone means a manifest served over https that points a download_url at +// http://same-host/... does NOT get the token — the scheme must match too. +// Deliberately not wrapped in try/catch: an invalid mirror throws here just as +// it does in getManifestUrl(), so both agree a bad mirror is fatal. +function getMirrorOrigin() { + return new URL(getMirror()).origin; } function isGitHubHost(host) { return (host === 'github.com' || @@ -98752,34 +98768,36 @@ function resolveRepoCoords() { const m = REPO_COORDS_RE.exec(mirror); if (m) return { owner: m[1], repo: m[2], branch: m[3] }; - // A raw.githubusercontent.com URL that doesn't parse is usually a branch - // name containing a slash, which is indistinguishable from a deeper path. - // Fetching still works, just anonymously and without the API rate limit. - if (!warnedMirrors.has(mirror) && - getMirrorHost() === 'raw.githubusercontent.com') { + // A raw.githubusercontent.com URL that doesn't parse is a branch name + // containing a slash (e.g. .../{owner}/{repo}/feature/riscv), which is + // indistinguishable from a deeper path. getMirror() succeeded above, so the + // URL is well-formed and this parse cannot throw. + const isRawGitHub = new URL(mirror).host === 'raw.githubusercontent.com'; + if (!warnedMirrors.has(mirror) && isRawGitHub) { warnedMirrors.add(mirror); - warning(`Could not parse owner/repo/branch out of mirror "${mirror}", so the manifest will be fetched by direct URL instead of the GitHub API. ` + - `Branch names containing '/' are not supported; use a branch without a slash to get the authenticated API rate limit.`); + warning(`Could not parse owner/repo/branch out of mirror "${mirror}", so the manifest will be fetched directly from the raw URL instead of through the GitHub REST API. ` + + `The request is still authenticated with your token, because raw.githubusercontent.com is a GitHub host. ` + + `Branch names containing '/' are not supported for the REST API path; use a branch without a slash if you want the manifest fetched through the API.`); } return null; } -// Mirror host with `mirror-token` set gets the token verbatim, so internal +// Mirror origin with `mirror-token` set gets the token verbatim, so internal // mirrors can choose their own scheme (Bearer, Basic, ...). GitHub hosts get // `token ${token}`. Anything else is anonymous — neither credential is sent to // a host the user didn't nominate. function authForUrl(url) { - let host; + let parsed; try { - host = new URL(url).host; + parsed = new URL(url); } catch { return undefined; } const mirrorToken = getMirrorToken(); - if (mirrorToken && host === getMirrorHost()) + if (mirrorToken && parsed.origin === getMirrorOrigin()) return mirrorToken; const token = getToken(); - if (token && isGitHubHost(host)) + if (token && isGitHubHost(parsed.host)) return `token ${token}`; return undefined; } @@ -103541,6 +103559,7 @@ function getCacheDistributor(packageManager, pythonVersion, cacheDependencyPath) + function isPyPyVersion(versionSpec) { return versionSpec.startsWith('pypy'); } @@ -103549,9 +103568,11 @@ function isGraalPyVersion(versionSpec) { } // `mirror` only redirects CPython distributions. PyPy and GraalPy resolve from // downloads.python.org and the GitHub releases API respectively, so warn rather -// than let the input look like it applied. +// than let the input look like it applied. Only warns when the user actually +// set a custom mirror: action.yml gives `mirror` a default, so a plain +// getInput() check would fire on every pypy-*/graalpy-* run. function warnIfMirrorUnsupported(versionSpec) { - if (!getInput('mirror')) { + if (!isMirrorCustomized()) { return; } const implementation = isPyPyVersion(versionSpec) ? 'PyPy' : 'GraalPy'; diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md index a6dbce6b..86c4f764 100644 --- a/docs/advanced-usage.md +++ b/docs/advanced-usage.md @@ -562,7 +562,7 @@ Point at an internal mirror with its own credential: Caveats: - `mirror` and `mirror-token` apply to **CPython only**. PyPy resolves from `downloads.python.org` and GraalPy from the GitHub releases API; both ignore these inputs, and the action warns if you set `mirror` alongside a `pypy-*` or `graalpy-*` version. -- Branch names containing `/` cannot be used with a `raw.githubusercontent.com` mirror, because `.../{owner}/{repo}/feature/riscv` is indistinguishable from a repo path. Such a mirror still works, but falls back to an anonymous direct GET with the 60/hr unauthenticated rate limit; the action warns when this happens. Use a branch without a slash to get the API path. +- Branch names containing `/` cannot be used with a `raw.githubusercontent.com` mirror, because `.../{owner}/{repo}/feature/riscv` is indistinguishable from a repo path. Such a mirror still works and is still authenticated with your `token` (raw.githubusercontent.com is a GitHub host), but the manifest is fetched directly from the raw URL rather than through the GitHub REST API; the action warns when this happens. Use a branch without a slash to get the REST API path. The `refs/heads/{branch}` form (for example `.../actions/python-versions/refs/heads/main`) is recognized and routes through the REST API. ### PyPy diff --git a/src/install-python.ts b/src/install-python.ts index 61c42ffe..67e958a8 100644 --- a/src/install-python.ts +++ b/src/install-python.ts @@ -14,9 +14,12 @@ const DEFAULT_REPO_NAME = 'python-versions'; const DEFAULT_REPO_BRANCH = 'main'; const DEFAULT_MIRROR = `https://raw.githubusercontent.com/${DEFAULT_REPO_OWNER}/${DEFAULT_REPO_NAME}/${DEFAULT_REPO_BRANCH}`; -// Matches https://raw.githubusercontent.com/{owner}/{repo}/{branch} +// Matches https://raw.githubusercontent.com/{owner}/{repo}/{branch} and the +// equivalent https://raw.githubusercontent.com/{owner}/{repo}/refs/heads/{branch} +// form, capturing the bare branch in both. The GitHub tree API wants the bare +// branch, so the optional refs/heads/ prefix is consumed, not captured. const REPO_COORDS_RE = - /^https:\/\/raw\.githubusercontent\.com\/([^/]+)\/([^/]+)\/([^/]+)\/?$/; + /^https:\/\/raw\.githubusercontent\.com\/([^/]+)\/([^/]+)\/(?:refs\/heads\/)?([^/]+)\/?$/; function getToken(): string { return core.getInput('token'); @@ -55,12 +58,26 @@ export function getManifestUrl(): string { return `${getMirror()}/versions-manifest.json`; } -function getMirrorHost(): string | undefined { - try { - return new URL(getMirror()).host; - } catch { - return undefined; - } +// Whether the user set `mirror` to something other than the built-in default. +// action.yml gives `mirror` a default, so core.getInput('mirror') is never +// empty; callers that want "did the user opt into a custom mirror" must compare +// against DEFAULT_MIRROR rather than test for a falsy input. Normalizes the +// same way getMirror() does but never throws, so a warning path can call it +// even when the mirror is malformed. +export function isMirrorCustomized(): boolean { + const input = core.getInput('mirror'); + if (!input) return false; + return input.trim().replace(/\/+$/, '') !== DEFAULT_MIRROR; +} + +// Origin (scheme + host + port) of the mirror, so `mirror-token` is matched +// against the exact origin the user nominated. Comparing origin rather than +// host alone means a manifest served over https that points a download_url at +// http://same-host/... does NOT get the token — the scheme must match too. +// Deliberately not wrapped in try/catch: an invalid mirror throws here just as +// it does in getManifestUrl(), so both agree a bad mirror is fatal. +function getMirrorOrigin(): string { + return new URL(getMirror()).origin; } function isGitHubHost(host: string): boolean { @@ -84,40 +101,40 @@ export function resolveRepoCoords(): { const m = REPO_COORDS_RE.exec(mirror); if (m) return {owner: m[1], repo: m[2], branch: m[3]}; - // A raw.githubusercontent.com URL that doesn't parse is usually a branch - // name containing a slash, which is indistinguishable from a deeper path. - // Fetching still works, just anonymously and without the API rate limit. - if ( - !warnedMirrors.has(mirror) && - getMirrorHost() === 'raw.githubusercontent.com' - ) { + // A raw.githubusercontent.com URL that doesn't parse is a branch name + // containing a slash (e.g. .../{owner}/{repo}/feature/riscv), which is + // indistinguishable from a deeper path. getMirror() succeeded above, so the + // URL is well-formed and this parse cannot throw. + const isRawGitHub = new URL(mirror).host === 'raw.githubusercontent.com'; + if (!warnedMirrors.has(mirror) && isRawGitHub) { warnedMirrors.add(mirror); core.warning( - `Could not parse owner/repo/branch out of mirror "${mirror}", so the manifest will be fetched by direct URL instead of the GitHub API. ` + - `Branch names containing '/' are not supported; use a branch without a slash to get the authenticated API rate limit.` + `Could not parse owner/repo/branch out of mirror "${mirror}", so the manifest will be fetched directly from the raw URL instead of through the GitHub REST API. ` + + `The request is still authenticated with your token, because raw.githubusercontent.com is a GitHub host. ` + + `Branch names containing '/' are not supported for the REST API path; use a branch without a slash if you want the manifest fetched through the API.` ); } return null; } -// Mirror host with `mirror-token` set gets the token verbatim, so internal +// Mirror origin with `mirror-token` set gets the token verbatim, so internal // mirrors can choose their own scheme (Bearer, Basic, ...). GitHub hosts get // `token ${token}`. Anything else is anonymous — neither credential is sent to // a host the user didn't nominate. function authForUrl(url: string): string | undefined { - let host: string; + let parsed: URL; try { - host = new URL(url).host; + parsed = new URL(url); } catch { return undefined; } const mirrorToken = getMirrorToken(); - if (mirrorToken && host === getMirrorHost()) return mirrorToken; + if (mirrorToken && parsed.origin === getMirrorOrigin()) return mirrorToken; const token = getToken(); - if (token && isGitHubHost(host)) return `token ${token}`; + if (token && isGitHubHost(parsed.host)) return `token ${token}`; return undefined; } diff --git a/src/setup-python.ts b/src/setup-python.ts index 41742fcc..8a3c7e37 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -2,6 +2,7 @@ import * as core from '@actions/core'; import * as finder from './find-python.js'; import * as finderPyPy from './find-pypy.js'; import * as finderGraalPy from './find-graalpy.js'; +import {isMirrorCustomized} from './install-python.js'; import * as path from 'path'; import * as os from 'os'; import {fileURLToPath} from 'url'; @@ -25,9 +26,11 @@ function isGraalPyVersion(versionSpec: string) { // `mirror` only redirects CPython distributions. PyPy and GraalPy resolve from // downloads.python.org and the GitHub releases API respectively, so warn rather -// than let the input look like it applied. +// than let the input look like it applied. Only warns when the user actually +// set a custom mirror: action.yml gives `mirror` a default, so a plain +// getInput() check would fire on every pypy-*/graalpy-* run. function warnIfMirrorUnsupported(versionSpec: string) { - if (!core.getInput('mirror')) { + if (!isMirrorCustomized()) { return; } const implementation = isPyPyVersion(versionSpec) ? 'PyPy' : 'GraalPy';