From 362bf588f85b52007b7d95eb0364b2da83118fb3 Mon Sep 17 00:00:00 2001 From: yiliang114 <1204183885@qq.com> Date: Thu, 7 May 2026 21:08:48 +0800 Subject: [PATCH] fix(installer): tighten verifier base-url + clarify test helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small refinements from the second review pass: - normalizeHttpsBaseUrl rejects everything except https, since real release URLs are always HTTPS. Accepting http previously would let an operator silently target a stale or attacker-controlled mirror. - Drop EXPECTED_RELEASE_ASSET_NAMES from the public exports; it was only used internally for the verification log line. - Rename the test helper standaloneChecksumContent to placeholderChecksumContent and document that the hashes in its output are placeholders — the remote verifier does not download archives or compare hashes, it only validates that SHA256SUMS lists the expected names and that each archive URL is reachable. The non-https rejection test now also covers `http://` in addition to the existing `file://` case. --- scripts/tests/install-script.test.js | 24 ++++++++++++++++++------ scripts/verify-installation-release.js | 12 +++++++----- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/scripts/tests/install-script.test.js b/scripts/tests/install-script.test.js index ad6593311..5b9c6a7e7 100644 --- a/scripts/tests/install-script.test.js +++ b/scripts/tests/install-script.test.js @@ -789,7 +789,7 @@ describe('standalone release packaging', () => { it('verifies release asset URLs from SHA256SUMS', async () => { const { EXPECTED_STANDALONE_ARCHIVE_NAMES, verifyReleaseBaseUrl } = await import(installationReleaseVerificationScriptUrl); - const checksumContent = standaloneChecksumContent( + const checksumContent = placeholderChecksumContent( EXPECTED_STANDALONE_ARCHIVE_NAMES, ); const fetchedUrls = []; @@ -827,7 +827,7 @@ describe('standalone release packaging', () => { it('falls back to ranged GET when remote HEAD is unavailable', async () => { const { EXPECTED_STANDALONE_ARCHIVE_NAMES, verifyReleaseBaseUrl } = await import(installationReleaseVerificationScriptUrl); - const checksumContent = standaloneChecksumContent( + const checksumContent = placeholderChecksumContent( EXPECTED_STANDALONE_ARCHIVE_NAMES, ); const observedMethods = []; @@ -856,7 +856,7 @@ describe('standalone release packaging', () => { it('rejects a release base URL with no archives reachable', async () => { const { EXPECTED_STANDALONE_ARCHIVE_NAMES, verifyReleaseBaseUrl } = await import(installationReleaseVerificationScriptUrl); - const checksumContent = standaloneChecksumContent( + const checksumContent = placeholderChecksumContent( EXPECTED_STANDALONE_ARCHIVE_NAMES, ); @@ -872,14 +872,22 @@ describe('standalone release packaging', () => { ).rejects.toThrow(/Release asset URL is not available/); }); - it('rejects a release base URL that is not http(s)', async () => { + it('rejects a release base URL that is not https', async () => { const { verifyReleaseBaseUrl } = await import( installationReleaseVerificationScriptUrl ); + // file:// must be rejected as a URL the verifier cannot reach safely. await expect(verifyReleaseBaseUrl('file:///tmp/release/')).rejects.toThrow( - /--base-url must use http or https/, + /--base-url must use https/, ); + + // Plain http must also be rejected even though it is technically a valid + // URL — release URLs are always HTTPS, and accepting http would let an + // operator silently target a stale or attacker-controlled mirror. + await expect( + verifyReleaseBaseUrl('http://example.com/release/'), + ).rejects.toThrow(/--base-url must use https/); }); it('rejects a runtime archive without a Node executable', () => { @@ -2091,7 +2099,11 @@ function writeStandaloneReleaseChecksums(outDir, archiveNames) { writeFileSync(path.join(outDir, 'SHA256SUMS'), `${lines.join('\n')}\n`); } -function standaloneChecksumContent(archiveNames) { +// Generates a SHA256SUMS-formatted string for the given archive names. The +// hash values are placeholders — the remote verifier (verifyReleaseBaseUrl) +// only checks that SHA256SUMS lists the expected entries and that each +// archive URL is reachable; it does not download archives or compare hashes. +function placeholderChecksumContent(archiveNames) { return `${archiveNames .map( (assetName) => diff --git a/scripts/verify-installation-release.js b/scripts/verify-installation-release.js index 48101c10a..227c3a508 100644 --- a/scripts/verify-installation-release.js +++ b/scripts/verify-installation-release.js @@ -112,7 +112,7 @@ async function verifyReleaseDirectory(dir) { async function verifyReleaseBaseUrl(baseUrl, options = {}) { const { fetchImpl = fetch } = options; - const normalizedBaseUrl = normalizeHttpBaseUrl(baseUrl); + const normalizedBaseUrl = normalizeHttpsBaseUrl(baseUrl); const checksumUrl = new URL('SHA256SUMS', normalizedBaseUrl).toString(); const checksums = parseSha256Sums(await fetchText(checksumUrl, fetchImpl)); assertExpectedChecksumEntries(checksums); @@ -188,15 +188,18 @@ async function fetchText(url, fetchImpl) { return response.text(); } -function normalizeHttpBaseUrl(baseUrl) { +function normalizeHttpsBaseUrl(baseUrl) { let parsed; try { parsed = new URL(baseUrl); } catch { fail(`--base-url must be a valid URL: ${baseUrl}`); } - if (parsed.protocol !== 'https:' && parsed.protocol !== 'http:') { - fail(`--base-url must use http or https: ${baseUrl}`); + // Real release URLs are always HTTPS. Tests use injected fetchImpl, so + // they don't need a real protocol. Rejecting non-https early prevents an + // operator from accidentally pointing the verifier at a plain-http mirror. + if (parsed.protocol !== 'https:') { + fail(`--base-url must use https: ${baseUrl}`); } if (!parsed.pathname.endsWith('/')) { parsed.pathname = `${parsed.pathname}/`; @@ -205,7 +208,6 @@ function normalizeHttpBaseUrl(baseUrl) { } export { - EXPECTED_RELEASE_ASSET_NAMES, EXPECTED_STANDALONE_ARCHIVE_NAMES, verifyReleaseBaseUrl, verifyReleaseDirectory,