diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 4d9873c3f..3e94d3662 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -97,8 +97,8 @@ PR review comments come from [CodeRabbit](https://docs.coderabbit.ai/) via [`.co Triggered by pushing a version tag (e.g., `v1.2.3`): -1. **`schema-release-compare`** — first job; compares this SHA’s `CURRENT_SCHEMA_VERSION` to the last **published** GitHub Release, writes the Actions step summary, and uploads a schema readme artifact. Job outputs feed installer notices and the draft release body. -2. **`prepare-github-release`** — **sole** creator of the draft GitHub release for the tag (`MESH_CLIENT_ALLOW_DRAFT_CREATE=1`), exports `release_id`, then prepends the schema compare note (via `RELEASE_ID`, not List Releases). On `workflow_dispatch`, the tag is resolved in the workflow from `package.json` and passed as `RELEASE_TAG` (not read inside the release API script — avoids CodeQL `js/file-access-to-http`). The schema note is rebuilt from `schema-release-compare` job outputs (`MESH_CLIENT_SCHEMA_*`), not from a downloaded markdown artifact (same CodeQL rule). +1. **`schema-release-compare`** — first job; compares this SHA’s `CURRENT_SCHEMA_VERSION` to the last **published** GitHub Release (paginated Releases API; highest semver among non-draft/non-prerelease rows; recovers `vX.Y.Z` from release **name** only when `tag_name` is missing or `untagged-*`), writes the Actions step summary, and uploads a schema readme artifact. Job outputs feed installer notices and the draft release body. +2. **`prepare-github-release`** — **sole** creator of the draft GitHub release for the tag (`MESH_CLIENT_ALLOW_DRAFT_CREATE=1`), exports `release_id` (reconstructed from validated digits before `GITHUB_OUTPUT` — CodeQL `js/http-to-file-access`), then prepends the schema compare note (via `RELEASE_ID`, not List Releases). On `workflow_dispatch`, the tag is resolved in the workflow from `package.json` and passed as `RELEASE_TAG` (not read inside the release API script — avoids CodeQL `js/file-access-to-http`). The schema note is rebuilt from `schema-release-compare` job outputs (`MESH_CLIENT_SCHEMA_*`), not from a downloaded markdown artifact (same CodeQL rule). 3. Installs Linux build dependencies (`libudev-dev`, `rpm`, …) on `ubuntu-latest` runners 4. Rebuilds native dependencies (`pnpm run rebuild`) 5. **Stamp CI build info** — `scripts/ci-write-build-info-env.mjs` writes `MESH_CLIENT_BUILD_INFO` (`buildChannel=release` + tag + Actions `runUrl`) into `$GITHUB_ENV` before `dist:*` so support-bundle `manifest.json` and startup logs identify an official release build (see [Build channel stamp](#build-channel-stamp-test-vs-release)). @@ -106,7 +106,7 @@ Triggered by pushing a version tag (e.g., `v1.2.3`): - `macos-latest` → `pnpm run dist:mac` - `ubuntu-latest` → `pnpm run dist:linux` - `windows-latest` → `pnpm run dist:win` -7. **`ci-upload-release-assets.mjs`** attaches installers / update metadata to the prepare `release_id` (never `POST /releases`). `finalize-github-release` still consolidates if anything external forked drafts. +7. **`ci-upload-release-assets.mjs`** attaches installers / update metadata to the prepare `release_id` (never `POST /releases`) via `gh api --input` path uploads (avoids CodeQL `js/file-access-to-http` from `readFile` → `fetch`). `finalize-github-release` still consolidates if anything external forked drafts. Linux packaging smoke (`verify-linux-packaging.mjs`) asserts `.deb` **Description** metadata is ASCII-only. See [Release Process](release-process.md). diff --git a/scripts/ci-ensure-github-draft-release.mjs b/scripts/ci-ensure-github-draft-release.mjs index cab4f2d2f..1330a1b4d 100644 --- a/scripts/ci-ensure-github-draft-release.mjs +++ b/scripts/ci-ensure-github-draft-release.mjs @@ -6,9 +6,13 @@ import { ensureGithubDraftRelease, resolveTag, resolveTargetCommitish, + trustedGithubReleaseId, } from './github-release-api.mjs'; /** + * Write prepare/wait `release_id` to GITHUB_OUTPUT. + * Id is reconstructed from validated digits so network JSON cannot taint the disk write + * (CodeQL `js/http-to-file-access`). * @param {string | undefined} githubOutput * @param {number | string} releaseId */ @@ -16,7 +20,8 @@ export function writeReleaseIdOutput(githubOutput, releaseId) { if (typeof githubOutput !== 'string' || !githubOutput) { return; } - appendFileSync(githubOutput, `release_id=${releaseId}\n`, 'utf8'); + const id = trustedGithubReleaseId(releaseId); + appendFileSync(githubOutput, `release_id=${id}\n`, 'utf8'); } async function main() { @@ -30,7 +35,9 @@ async function main() { allowCreate, }); writeReleaseIdOutput(process.env.GITHUB_OUTPUT, release.id); - console.debug(`[ci-ensure-github-draft-release] release_id=${release.id}`); + console.debug( + `[ci-ensure-github-draft-release] release_id=${trustedGithubReleaseId(release.id)}`, + ); } const entry = process.argv[1] ? pathToFileURL(process.argv[1]).href : ''; diff --git a/scripts/ci-ensure-github-draft-release.test.mjs b/scripts/ci-ensure-github-draft-release.test.mjs index cb4c3075f..f913c7fff 100644 --- a/scripts/ci-ensure-github-draft-release.test.mjs +++ b/scripts/ci-ensure-github-draft-release.test.mjs @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { + assertSafeReleaseAssetName, assertSafeReleaseTag, consolidateReleases, ensureGithubDraftRelease, @@ -7,8 +8,15 @@ import { normalizeDraftReleasesForTag, pickCanonicalRelease, resolveTag, + trustedGithubReleaseId, + uploadOrReplaceReleaseAsset, + uploadReleaseAssetFromFile, waitForGithubDraftRelease, } from './github-release-api.mjs'; +import { writeReleaseIdOutput } from './ci-ensure-github-draft-release.mjs'; +import { mkdtempSync, readFileSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; const TAG = 'v5.21.0'; @@ -30,6 +38,158 @@ describe('assertSafeReleaseTag', () => { }); }); +describe('trustedGithubReleaseId', () => { + it('rebuilds a positive integer from digits', () => { + expect(trustedGithubReleaseId(368221738)).toBe(368221738); + expect(trustedGithubReleaseId('99')).toBe(99); + }); + + it('rejects zero, negatives, and non-digits', () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined); + trustedGithubReleaseId(0); + expect(exitSpy).toHaveBeenCalledWith(1); + exitSpy.mockClear(); + trustedGithubReleaseId('-1'); + expect(exitSpy).toHaveBeenCalledWith(1); + exitSpy.mockClear(); + trustedGithubReleaseId('12ab'); + expect(exitSpy).toHaveBeenCalledWith(1); + exitSpy.mockRestore(); + }); + + it('rejects ids above Number.MAX_SAFE_INTEGER', () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined); + trustedGithubReleaseId('9007199254740993'); + expect(exitSpy).toHaveBeenCalledWith(1); + exitSpy.mockRestore(); + }); +}); + +describe('assertSafeReleaseAssetName', () => { + it('accepts basename-only names', () => { + expect(assertSafeReleaseAssetName('mesh-client.dmg')).toBe('mesh-client.dmg'); + }); + + it('rejects path separators', () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined); + assertSafeReleaseAssetName('../evil.bin'); + expect(exitSpy).toHaveBeenCalledWith(1); + exitSpy.mockRestore(); + }); +}); + +describe('writeReleaseIdOutput', () => { + it('writes a trusted release_id line', () => { + const dir = mkdtempSync(path.join(tmpdir(), 'mesh-gh-out-')); + const out = path.join(dir, 'github_output'); + writeFileSync(out, ''); + writeReleaseIdOutput(out, '368221738'); + expect(readFileSync(out, 'utf8')).toBe('release_id=368221738\n'); + }); +}); + +describe('uploadReleaseAssetFromFile', () => { + it('invokes gh api --input with the file path (no JS readFile→fetch)', () => { + const dir = mkdtempSync(path.join(tmpdir(), 'mesh-gh-upload-')); + const filePath = path.join(dir, 'a.deb'); + writeFileSync(filePath, 'bytes'); + const execFile = vi.fn(() => JSON.stringify({ id: 1, name: 'a.deb' })); + const result = uploadReleaseAssetFromFile(9, 'a.deb', filePath, 'token', { + execFileSync: execFile, + }); + expect(result).toEqual({ id: 1, name: 'a.deb' }); + expect(execFile).toHaveBeenCalledTimes(1); + const [cmd, args] = execFile.mock.calls[0]; + expect(cmd).toBe('gh'); + expect(args).toContain('--input'); + expect(args).toContain(filePath); + expect(args.some((a) => String(a).includes('/releases/9/assets'))).toBe(true); + }); +}); + +describe('uploadOrReplaceReleaseAsset', () => { + it('validates releaseId and fileName before lookup or delete', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation((code) => { + throw new Error(`exit:${code}`); + }); + const getReleaseById = vi.fn(); + const deleteAsset = vi.fn(); + await expect( + uploadOrReplaceReleaseAsset({ + releaseId: 'not-a-number', + token: 'token', + fileName: 'a.deb', + bytes: new Uint8Array([1]), + getReleaseById, + deleteAsset, + log: () => {}, + }), + ).rejects.toThrow(/exit:1/); + expect(getReleaseById).not.toHaveBeenCalled(); + expect(deleteAsset).not.toHaveBeenCalled(); + exitSpy.mockRestore(); + }); + + it('restores the prior asset when replacement upload fails', async () => { + const dir = mkdtempSync(path.join(tmpdir(), 'mesh-replace-')); + const filePath = path.join(dir, 'a.deb'); + writeFileSync(filePath, 'new'); + const priorBytes = new Uint8Array([9, 9, 9]); + const restored = []; + const logs = []; + const exitSpy = vi.spyOn(process, 'exit').mockImplementation((code) => { + throw new Error(`exit:${code}`); + }); + + await expect( + uploadOrReplaceReleaseAsset({ + releaseId: 42, + token: 'token', + fileName: 'a.deb', + filePath, + existingAssets: [{ id: 7, name: 'a.deb' }], + downloadAsset: async () => priorBytes, + deleteAsset: async () => {}, + uploadFromFile: () => { + throw new Error('gh upload exploded'); + }, + uploadBytes: async (_id, name, bytes) => { + restored.push({ name, bytes }); + return { id: 99, name }; + }, + log: (message) => logs.push(message), + }), + ).rejects.toThrow(/exit:1/); + + expect(restored).toEqual([{ name: 'a.deb', bytes: priorBytes }]); + expect(logs.some((line) => line.includes('Restored prior asset a.deb'))).toBe(true); + exitSpy.mockRestore(); + }); + + it('does not delete a prior asset when the upload path is missing', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation((code) => { + throw new Error(`exit:${code}`); + }); + const deleteAsset = vi.fn(); + const downloadAsset = vi.fn(); + await expect( + uploadOrReplaceReleaseAsset({ + releaseId: 42, + token: 'token', + fileName: 'a.deb', + filePath: path.join(mkdtempSync(path.join(tmpdir(), 'mesh-miss-')), 'a.deb'), + existingAssets: [{ id: 7, name: 'a.deb' }], + deleteAsset, + downloadAsset, + log: () => {}, + }), + ).rejects.toThrow(/exit:1/); + expect(downloadAsset).not.toHaveBeenCalled(); + expect(deleteAsset).not.toHaveBeenCalled(); + exitSpy.mockRestore(); + }); +}); + describe('resolveTag', () => { it('uses RELEASE_TAG when set', () => { const tag = resolveTag([], { RELEASE_TAG: 'v5.21.0' }); diff --git a/scripts/ci-schema-release-compare.mjs b/scripts/ci-schema-release-compare.mjs index 0c80b12b9..006def3f7 100644 --- a/scripts/ci-schema-release-compare.mjs +++ b/scripts/ci-schema-release-compare.mjs @@ -115,6 +115,58 @@ export function trustedReleaseTag(tag) { return `v${Number(m[1])}.${Number(m[2])}.${Number(m[3])}`; } +/** Release display names are X.Y.Z (no leading v) on GitHub Releases. */ +export const SAFE_RELEASE_NAME_RE = /^(\d+)\.(\d+)\.(\d+)$/; + +/** + * Compare two trusted `vX.Y.Z` tags. Negative if a < b. + * @param {string} a + * @param {string} b + */ +export function compareReleaseTags(a, b) { + const pa = trustedReleaseTag(a).slice(1).split('.').map(Number); + const pb = trustedReleaseTag(b).slice(1).split('.').map(Number); + for (let i = 0; i < 3; i += 1) { + if (pa[i] !== pb[i]) return pa[i] - pb[i]; + } + return 0; +} + +/** + * Map a GitHub Releases API row to a trusted git tag for schema lookup. + * Skips drafts/prereleases. Accepts normal `tag_name: vX.Y.Z`, and published + * releases that lost their tag (`untagged-*` or empty) when `name` is still `X.Y.Z` + * (seen after draft-fork / publish races — otherwise test builds warn against + * an older release/schema). + * + * @param {{ tag_name?: unknown, name?: unknown, draft?: unknown, prerelease?: unknown } | null | undefined} release + * @returns {string | null} + */ +export function publishedReleaseGitTag(release) { + if (!release || release.draft === true || release.prerelease === true) { + return null; + } + const tag = release.tag_name; + if (typeof tag === 'string' && /^v\d+\.\d+\.\d+$/.test(tag)) { + return trustedReleaseTag(tag); + } + // Name fallback only when tag is missing or GitHub's untagged-* placeholder — + // not for arbitrary invalid tags (e.g. tag_name "broken" + name "5.27.0"). + const tagMissing = tag == null || tag === ''; + const tagUntagged = typeof tag === 'string' && /^untagged-/i.test(tag); + if (!tagMissing && !tagUntagged) { + return null; + } + const name = release.name; + if (typeof name === 'string') { + const m = SAFE_RELEASE_NAME_RE.exec(name); + if (m) { + return trustedReleaseTag(`v${Number(m[1])}.${Number(m[2])}.${Number(m[3])}`); + } + } + return null; +} + /** * Coerce a schema version to a trusted positive integer. * @param {unknown} value @@ -142,6 +194,88 @@ export function readSchemaVersionFromGitTag(tag, cwd = ROOT) { return trustedSchemaVersion(parseCurrentSchemaVersion(source)); } +/** + * Pick the highest-semver published release tag from a GitHub Releases list. + * @param {Array<{ tag_name?: unknown, name?: unknown, draft?: unknown, prerelease?: unknown }>} releases + * @param {string} [excludeTag] + * @returns {string | null} + */ +export function pickLatestPublishedReleaseTag(releases, excludeTag) { + if (!Array.isArray(releases)) { + return null; + } + /** @type {string[]} */ + const candidates = []; + for (const release of releases) { + const tag = publishedReleaseGitTag(release); + if (!tag) continue; + if (excludeTag && tag === excludeTag) continue; + candidates.push(tag); + } + if (candidates.length === 0) { + return null; + } + // Prefer highest semver — API order is not reliable when drafts / untagged + // rows interleave with published releases. + candidates.sort((a, b) => compareReleaseTags(b, a)); + return candidates[0]; +} + +/** + * Parse the next page URL from a GitHub `Link` response header. + * @param {string | null | undefined} linkHeader + * @returns {string | null} + */ +export function parseGithubLinkNext(linkHeader) { + if (typeof linkHeader !== 'string' || !linkHeader) { + return null; + } + for (const segment of linkHeader.split(',')) { + const match = segment.match(/<([^>]+)>\s*;\s*rel="next"/i); + if (match) { + return match[1]; + } + } + return null; +} + +/** + * Fetch every Releases API page (follows `Link: rel="next"`). + * @param {{ + * owner?: string, + * repo?: string, + * headers?: Record, + * fetchImpl?: typeof fetch, + * }} [opts] + * @returns {Promise>} + */ +export async function fetchAllGithubReleases(opts = {}) { + const owner = opts.owner ?? 'Colorado-Mesh'; + const repo = opts.repo ?? 'mesh-client'; + const headers = opts.headers ?? { + Accept: 'application/vnd.github+json', + 'X-GitHub-Api-Version': '2022-11-28', + }; + const fetchImpl = opts.fetchImpl ?? fetch; + + /** @type {Array<{ tag_name?: string, name?: string, draft?: boolean, prerelease?: boolean }>} */ + const releases = []; + let url = `https://api.github.com/repos/${owner}/${repo}/releases?per_page=100`; + while (url) { + const listRes = await fetchImpl(url, { headers }); + if (!listRes.ok) { + throw new Error(`List releases failed (${listRes.status}): ${await listRes.text()}`); + } + const page = await listRes.json(); + if (!Array.isArray(page)) { + throw new Error('Unexpected releases list payload'); + } + releases.push(...page); + url = parseGithubLinkNext(listRes.headers.get('link')); + } + return releases; +} + /** * @param {{ token?: string, owner?: string, repo?: string, excludeTag?: string }} [opts] * @returns {Promise<{ tag: string, schema: number } | null>} @@ -158,30 +292,12 @@ export async function fetchLatestPublishedReleaseSchema(opts = {}) { headers.Authorization = `Bearer ${token}`; } - const listUrl = `https://api.github.com/repos/${owner}/${repo}/releases?per_page=20`; - const listRes = await fetch(listUrl, { headers }); - if (!listRes.ok) { - throw new Error(`List releases failed (${listRes.status}): ${await listRes.text()}`); - } - /** @type {Array<{ tag_name?: string, draft?: boolean, prerelease?: boolean }>} */ - const releases = await listRes.json(); - if (!Array.isArray(releases)) { - throw new Error('Unexpected releases list payload'); - } - - const candidate = releases.find((r) => { - if (!r || r.draft === true || r.prerelease === true) return false; - const tag = r.tag_name; - if (typeof tag !== 'string' || !/^v\d+\.\d+\.\d+$/.test(tag)) return false; - if (opts.excludeTag && tag === opts.excludeTag) return false; - return true; - }); - if (!candidate?.tag_name) { + const releases = await fetchAllGithubReleases({ owner, repo, headers }); + const tag = pickLatestPublishedReleaseTag(releases, opts.excludeTag); + if (!tag) { return null; } - // Rebuild tag/schema from validated digits so later disk writes are not network-tainted. - const tag = trustedReleaseTag(candidate.tag_name); try { execFileSync('git', ['fetch', '--depth', '1', 'origin', `refs/tags/${tag}:refs/tags/${tag}`], { cwd: ROOT, diff --git a/scripts/ci-schema-release-compare.test.mjs b/scripts/ci-schema-release-compare.test.mjs index ca63c14f1..9b8269295 100644 --- a/scripts/ci-schema-release-compare.test.mjs +++ b/scripts/ci-schema-release-compare.test.mjs @@ -3,9 +3,14 @@ import os from 'node:os'; import path from 'node:path'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { + compareReleaseTags, + fetchAllGithubReleases, formatSchemaCompareMarkdown, isSchemaBumped, parseCurrentSchemaVersion, + parseGithubLinkNext, + pickLatestPublishedReleaseTag, + publishedReleaseGitTag, runSchemaReleaseCompare, trustedReleaseTag, trustedSchemaVersion, @@ -35,6 +40,159 @@ describe('trustedReleaseTag / trustedSchemaVersion', () => { }); }); +describe('publishedReleaseGitTag', () => { + it('uses tag_name when it is a normal vX.Y.Z release', () => { + expect( + publishedReleaseGitTag({ + tag_name: 'v5.25.0', + name: '5.25.0', + draft: false, + prerelease: false, + }), + ).toBe('v5.25.0'); + }); + + it('recovers vX.Y.Z from release name when tag_name is untagged-*', () => { + // Regression: published 5.27.0 lost its git tag after draft-fork races; + // compare used to skip it and warn against older v5.25.0 / schema 47. + expect( + publishedReleaseGitTag({ + tag_name: 'untagged-56bb16db7c14eda58971', + name: '5.27.0', + draft: false, + prerelease: false, + }), + ).toBe('v5.27.0'); + }); + + it('rejects a numeric name paired with an unrelated invalid tag_name', () => { + expect( + publishedReleaseGitTag({ + tag_name: 'broken', + name: '5.27.0', + draft: false, + prerelease: false, + }), + ).toBeNull(); + }); + + it('skips drafts and prereleases even with a valid tag_name', () => { + expect( + publishedReleaseGitTag({ + tag_name: 'v5.26.0', + name: '5.26.0', + draft: true, + prerelease: false, + }), + ).toBeNull(); + expect( + publishedReleaseGitTag({ + tag_name: 'v5.21.0', + name: '5.21.0', + draft: false, + prerelease: true, + }), + ).toBeNull(); + }); +}); + +describe('compareReleaseTags', () => { + it('orders semver tags', () => { + expect(compareReleaseTags('v5.25.0', 'v5.27.0')).toBeLessThan(0); + expect(compareReleaseTags('v5.27.0', 'v5.25.0')).toBeGreaterThan(0); + expect(compareReleaseTags('v5.27.0', 'v5.27.0')).toBe(0); + }); +}); + +describe('pickLatestPublishedReleaseTag', () => { + it('prefers untagged published 5.27.0 over older tagged releases and skips drafts', () => { + // Mirrors production list shape after the v5.27.0 publish race. + const tag = pickLatestPublishedReleaseTag([ + { tag_name: 'v5.26.0', name: '5.26.0', draft: true, prerelease: false }, + { + tag_name: 'untagged-56bb16db7c14eda58971', + name: '5.27.0', + draft: false, + prerelease: false, + }, + { tag_name: 'v5.25.0', name: '5.25.0', draft: false, prerelease: false }, + ]); + expect(tag).toBe('v5.27.0'); + }); + + it('honors excludeTag for the release being published', () => { + const tag = pickLatestPublishedReleaseTag( + [ + { tag_name: 'v5.28.0', name: '5.28.0', draft: false, prerelease: false }, + { tag_name: 'v5.27.0', name: '5.27.0', draft: false, prerelease: false }, + ], + 'v5.28.0', + ); + expect(tag).toBe('v5.27.0'); + }); + + it('selects the highest version when it appears after the first page worth of rows', () => { + /** @type {Array<{ tag_name: string, name: string, draft: boolean, prerelease: boolean }>} */ + const page1 = Array.from({ length: 30 }, (_, i) => { + const patch = 30 - i; + return { + tag_name: `v5.0.${patch}`, + name: `5.0.${patch}`, + draft: false, + prerelease: false, + }; + }); + const all = [ + ...page1, + { tag_name: 'v5.99.0', name: '5.99.0', draft: false, prerelease: false }, + ]; + expect(pickLatestPublishedReleaseTag(all)).toBe('v5.99.0'); + }); +}); + +describe('parseGithubLinkNext / fetchAllGithubReleases', () => { + it('parses rel=next from a GitHub Link header', () => { + expect( + parseGithubLinkNext( + '; rel="next", ; rel="last"', + ), + ).toBe('https://api.github.com/repos/o/r/releases?page=2'); + expect(parseGithubLinkNext(null)).toBeNull(); + }); + + it('accumulates releases across pages so a late high version is visible', async () => { + const page1 = Array.from({ length: 30 }, (_, i) => ({ + tag_name: `v4.0.${i}`, + name: `4.0.${i}`, + draft: false, + prerelease: false, + })); + const page2 = [ + { tag_name: 'v9.0.0', name: '9.0.0', draft: false, prerelease: false }, + { tag_name: 'v5.0.0', name: '5.0.0', draft: false, prerelease: false }, + ]; + const fetchImpl = vi.fn(async (url) => { + if (String(url).includes('page=2')) { + return new Response(JSON.stringify(page2), { + status: 200, + headers: { link: '' }, + }); + } + return new Response(JSON.stringify(page1), { + status: 200, + headers: { + link: '; rel="next"', + }, + }); + }); + + const releases = await fetchAllGithubReleases({ fetchImpl }); + expect(releases).toHaveLength(32); + expect(pickLatestPublishedReleaseTag(releases)).toBe('v9.0.0'); + expect(fetchImpl).toHaveBeenCalledTimes(2); + }); +}); + describe('isSchemaBumped', () => { it('is true only when current is greater than previous', () => { expect(isSchemaBumped(48, 47)).toBe(true); diff --git a/scripts/ci-upload-release-assets.mjs b/scripts/ci-upload-release-assets.mjs index 8f8a70f52..90779d8fb 100644 --- a/scripts/ci-upload-release-assets.mjs +++ b/scripts/ci-upload-release-assets.mjs @@ -2,20 +2,28 @@ /** * Upload local files to an existing GitHub release by id. * Never creates a release (prevents duplicate draft forks from electron-builder / softprops). + * + * Files are passed as paths into `gh api --input` (via uploadOrReplaceReleaseAsset) so this + * process never joins readFile → fetch (CodeQL `js/file-access-to-http`). */ -import { readFileSync, globSync, statSync } from 'node:fs'; +import { globSync, statSync } from 'node:fs'; import path from 'node:path'; import { pathToFileURL } from 'node:url'; -import { authToken, fail, getRelease, uploadOrReplaceReleaseAsset } from './github-release-api.mjs'; +import { + assertReadableReleaseUploadFile, + assertSafeReleaseAssetName, + authToken, + fail, + getRelease, + trustedGithubReleaseId, + uploadOrReplaceReleaseAsset, +} from './github-release-api.mjs'; /** * @param {string | undefined} raw */ export function parseReleaseId(raw) { - if (typeof raw !== 'string' || !/^\d+$/.test(raw)) { - fail(`RELEASE_ID must be a numeric GitHub release id (got ${JSON.stringify(raw)})`); - } - return Number(raw); + return trustedGithubReleaseId(raw); } /** @@ -78,24 +86,23 @@ export function findDuplicateBasenames(files) { /** * @param {{ - * releaseId: number, + * releaseId: number | string, * token: string, * files: string[], * get?: typeof getRelease, * upload?: typeof uploadOrReplaceReleaseAsset, - * readFile?: (path: string) => Uint8Array, * log?: (...args: unknown[]) => void, * }} opts */ export async function uploadReleaseAssets(opts) { const get = opts.get ?? getRelease; const upload = opts.upload ?? uploadOrReplaceReleaseAsset; - const readFile = opts.readFile ?? ((filePath) => new Uint8Array(readFileSync(filePath))); const log = opts.log ?? console.debug; + const releaseId = trustedGithubReleaseId(opts.releaseId); - const release = await get(opts.releaseId, opts.token); + const release = await get(releaseId, opts.token); if (release.draft !== true) { - fail(`Release ${opts.releaseId} is not a draft; refusing to upload`); + fail(`Release ${releaseId} is not a draft; refusing to upload`); return 0; } @@ -110,16 +117,15 @@ export async function uploadReleaseAssets(opts) { let uploaded = 0; for (const filePath of opts.files) { - const fileName = path.basename(filePath); - const bytes = readFile(filePath); - log( - `[ci-upload-release-assets] Uploading ${fileName} (${bytes.byteLength} bytes) → release ${opts.releaseId}`, - ); + const fileName = assertSafeReleaseAssetName(path.basename(filePath)); + // Validate readability before upload/replace can delete a prior asset. + assertReadableReleaseUploadFile(filePath, fileName); + log(`[ci-upload-release-assets] Uploading ${fileName} → release ${releaseId}`); await upload({ - releaseId: opts.releaseId, + releaseId, token: opts.token, fileName, - bytes, + filePath, existingAssets, log, }); @@ -128,7 +134,7 @@ export async function uploadReleaseAssets(opts) { uploaded += 1; } - log(`[ci-upload-release-assets] Uploaded ${uploaded} asset(s) to release ${opts.releaseId}`); + log(`[ci-upload-release-assets] Uploaded ${uploaded} asset(s) to release ${releaseId}`); return uploaded; } diff --git a/scripts/ci-upload-release-assets.test.mjs b/scripts/ci-upload-release-assets.test.mjs index e75fdb3c6..8e70157ec 100644 --- a/scripts/ci-upload-release-assets.test.mjs +++ b/scripts/ci-upload-release-assets.test.mjs @@ -85,26 +85,57 @@ describe('uploadReleaseAssets', () => { exitSpy.mockRestore(); }); - it('uploads each file to a draft release', async () => { + it('uploads each file to a draft release by path', async () => { + const dir = mkdtempSync(path.join(tmpdir(), 'mesh-upload-ok-')); + const a = path.join(dir, 'a.deb'); + const b = path.join(dir, 'b.yml'); + writeFileSync(a, 'a'); + writeFileSync(b, 'b'); const uploads = []; const count = await uploadReleaseAssets({ releaseId: 9, token: 'token', - files: ['/tmp/a.deb', '/tmp/b.yml'], + files: [a, b], get: async () => ({ id: 9, draft: true, assets: [{ id: 3, name: 'a.deb' }], }), - readFile: (filePath) => new Uint8Array(Buffer.from(path.basename(filePath))), upload: async (opts) => { - uploads.push(opts.fileName); + uploads.push({ fileName: opts.fileName, filePath: opts.filePath }); return { id: 1, name: opts.fileName }; }, log: () => {}, }); expect(count).toBe(2); - expect(uploads).toEqual(['a.deb', 'b.yml']); + expect(uploads).toEqual([ + { fileName: 'a.deb', filePath: a }, + { fileName: 'b.yml', filePath: b }, + ]); + }); + + it('refuses a missing upload path before calling upload (preserves prior assets)', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation((code) => { + throw new Error(`exit:${code}`); + }); + const upload = vi.fn(); + const missing = path.join(mkdtempSync(path.join(tmpdir(), 'mesh-upload-miss-')), 'gone.deb'); + await expect( + uploadReleaseAssets({ + releaseId: 9, + token: 'token', + files: [missing], + get: async () => ({ + id: 9, + draft: true, + assets: [{ id: 3, name: 'gone.deb' }], + }), + upload, + log: () => {}, + }), + ).rejects.toThrow(/exit:1/); + expect(upload).not.toHaveBeenCalled(); + exitSpy.mockRestore(); }); }); diff --git a/scripts/github-release-api.mjs b/scripts/github-release-api.mjs index 2c5ad8a43..bebd3e354 100644 --- a/scripts/github-release-api.mjs +++ b/scripts/github-release-api.mjs @@ -3,6 +3,10 @@ * Shared GitHub release helpers for CI ensure + manual consolidation. */ +import { execFileSync } from 'node:child_process'; +import { statSync } from 'node:fs'; +import path from 'node:path'; + export const OWNER = 'Colorado-Mesh'; export const REPO = 'mesh-client'; export const API_ROOT = `https://api.github.com/repos/${OWNER}/${REPO}`; @@ -10,6 +14,9 @@ export const API_ROOT = `https://api.github.com/repos/${OWNER}/${REPO}`; /** Release tags must be vX.Y.Z — validated before any GitHub API call (CodeQL file-access-to-http). */ export const SAFE_RELEASE_TAG_RE = /^v\d+\.\d+\.\d+$/; +/** Positive GitHub release ids as decimal digits (validated before Number conversion). */ +export const SAFE_GITHUB_RELEASE_ID_RE = /^([1-9]\d{0,18})$/; + export function versionFromTag(tag) { return tag.startsWith('v') ? tag.slice(1) : tag; } @@ -38,6 +45,78 @@ export function assertSafeReleaseTag(tag) { return tag; } +/** + * Reconstruct a trusted positive release id from validated digits (breaks CodeQL + * `js/http-to-file-access` taint from GitHub release JSON → GITHUB_OUTPUT writes). + * Rejects values outside Number.MAX_SAFE_INTEGER so URL/log interpolation stays exact. + * @param {unknown} value + * @returns {number} + */ +export function trustedGithubReleaseId(value) { + const m = SAFE_GITHUB_RELEASE_ID_RE.exec(String(value ?? '')); + if (!m) { + fail(`GitHub release id must be a positive integer (got ${JSON.stringify(value)})`); + return /** @type {never} */ (0); + } + const id = Number(m[1]); + if (!Number.isSafeInteger(id) || id < 1) { + fail(`GitHub release id must be <= Number.MAX_SAFE_INTEGER (got ${JSON.stringify(value)})`); + return /** @type {never} */ (0); + } + return id; +} + +/** + * Basename-only asset names for upload URLs / gh --input (no path separators). + * @param {unknown} fileName + * @returns {string} + */ +export function assertSafeReleaseAssetName(fileName) { + if ( + typeof fileName !== 'string' || + !fileName || + fileName === '.' || + fileName === '..' || + /[\\/]/.test(fileName) || + // eslint-disable-next-line no-control-regex -- reject ASCII controls in asset names + /[\x00-\x1F\x7F]/.test(fileName) + ) { + fail(`Unsafe release asset name: ${JSON.stringify(fileName)}`); + return /** @type {never} */ (''); + } + return fileName; +} + +/** + * Ensure a disk upload path is a readable regular file whose basename matches `fileName`. + * Call before deleting a prior release asset so a missing path cannot orphan the old asset. + * @param {unknown} filePath + * @param {string} fileName already validated via assertSafeReleaseAssetName + * @returns {string} + */ +export function assertReadableReleaseUploadFile(filePath, fileName) { + const name = assertSafeReleaseAssetName(fileName); + if (typeof filePath !== 'string' || !filePath) { + fail(`Upload file path is required for asset ${name}`); + return /** @type {never} */ (''); + } + if (path.basename(filePath) !== name) { + fail(`Asset name ${JSON.stringify(name)} must match basename of ${JSON.stringify(filePath)}`); + return /** @type {never} */ (''); + } + try { + const st = statSync(filePath); + if (!st.isFile()) { + fail(`Upload path is not a regular file: ${JSON.stringify(filePath)}`); + return /** @type {never} */ (''); + } + } catch { + fail(`Upload file not readable for asset ${name}: ${JSON.stringify(filePath)}`); + return /** @type {never} */ (''); + } + return filePath; +} + export function resolveTag(argv, env) { const flagIndex = argv.indexOf('--tag'); if (flagIndex >= 0 && argv[flagIndex + 1]) { @@ -260,8 +339,16 @@ export async function getRelease(releaseId, token) { return json; } -export async function uploadReleaseAsset(releaseId, fileName, bytes, token) { - const uploadUrl = `https://uploads.github.com/repos/${OWNER}/${REPO}/releases/${releaseId}/assets?name=${encodeURIComponent(fileName)}`; +export async function uploadReleaseAsset( + releaseId, + fileName, + bytes, + token, + { throwOnError = false } = {}, +) { + const id = trustedGithubReleaseId(releaseId); + const name = assertSafeReleaseAssetName(fileName); + const uploadUrl = `https://uploads.github.com/repos/${OWNER}/${REPO}/releases/${id}/assets?name=${encodeURIComponent(name)}`; const response = await fetch(uploadUrl, { method: 'POST', headers: { @@ -284,23 +371,104 @@ export async function uploadReleaseAsset(releaseId, fileName, bytes, token) { } if (!response.ok) { - fail( - `Upload asset ${fileName} to release ${releaseId} failed (${response.status}): ${json?.message ?? response.statusText}`, - ); + const message = `Upload asset ${name} to release ${id} failed (${response.status}): ${json?.message ?? response.statusText}`; + if (throwOnError) { + throw new Error(message); + } + fail(message); } return json; } +/** + * Upload a local file via `gh api --input` so JS never joins readFile → fetch + * (CodeQL `js/file-access-to-http`). Used by CI packaging uploads; consolidate still + * uses in-memory bytes from `downloadReleaseAsset` (network → network). + * + * @param {number | string} releaseId + * @param {string} fileName + * @param {string} filePath + * @param {string} token + * @param {{ execFileSync?: typeof execFileSync, throwOnError?: boolean }} [opts] + */ +export function uploadReleaseAssetFromFile( + releaseId, + fileName, + filePath, + token, + { execFileSync: execFile = execFileSync, throwOnError = false } = {}, +) { + const id = trustedGithubReleaseId(releaseId); + const name = assertSafeReleaseAssetName(fileName); + assertReadableReleaseUploadFile(filePath, name); + + const uploadUrl = `https://uploads.github.com/repos/${OWNER}/${REPO}/releases/${id}/assets?name=${encodeURIComponent(name)}`; + let stdout; + try { + stdout = execFile( + 'gh', + [ + 'api', + '--method', + 'POST', + '-H', + 'Accept: application/vnd.github+json', + '-H', + 'Content-Type: application/octet-stream', + '-H', + 'X-GitHub-Api-Version: 2022-11-28', + uploadUrl, + '--input', + filePath, + ], + { + env: { ...process.env, GH_TOKEN: token, GITHUB_TOKEN: token }, + encoding: 'utf8', + maxBuffer: 32 * 1024 * 1024, + }, + ); + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + const message = `Upload asset ${name} to release ${id} failed: ${detail}`; + if (throwOnError) { + throw new Error(message, { cause: error }); + } + fail(message); + return /** @type {never} */ (undefined); + } + + if (typeof stdout === 'string' && stdout.trim()) { + try { + return JSON.parse(stdout); + } catch { + return { name }; + } + } + return { name }; +} + /** * Upload (or replace) a single asset on an existing release. Never creates a release. + * Prefer `filePath` for disk uploads (CodeQL); use `bytes` for in-memory consolidate moves. + * + * When replacing, the prior asset bytes are cached before delete. If the replacement + * upload fails, those bytes are re-uploaded (best-effort restore). Failure point: if + * both replacement and restore fail, the prior asset is gone — job exits non-zero. + * * @param {{ - * releaseId: number, + * releaseId: number | string, * token: string, * fileName: string, - * bytes: Uint8Array, + * bytes?: Uint8Array, + * filePath?: string, * existingAssets?: Array<{ id: number, name: string }>, * log?: (...args: unknown[]) => void, + * uploadFromFile?: typeof uploadReleaseAssetFromFile, + * downloadAsset?: typeof downloadReleaseAsset, + * uploadBytes?: typeof uploadReleaseAsset, + * deleteAsset?: typeof deleteReleaseAsset, + * getReleaseById?: typeof getRelease, * }} opts */ export async function uploadOrReplaceReleaseAsset({ @@ -308,16 +476,64 @@ export async function uploadOrReplaceReleaseAsset({ token, fileName, bytes, + filePath, existingAssets, log = console.debug, + uploadFromFile = uploadReleaseAssetFromFile, + downloadAsset = downloadReleaseAsset, + uploadBytes = uploadReleaseAsset, + deleteAsset = deleteReleaseAsset, + getReleaseById = getRelease, }) { - const assets = existingAssets ?? (await getRelease(releaseId, token)).assets ?? []; - const prior = assets.find((asset) => asset.name === fileName); + const id = trustedGithubReleaseId(releaseId); + const name = assertSafeReleaseAssetName(fileName); + const usePath = typeof filePath === 'string' && filePath !== ''; + + // Validate path (or require bytes) before any delete so a missing file cannot + // remove the existing release asset. + if (usePath) { + assertReadableReleaseUploadFile(filePath, name); + } else if (bytes == null) { + fail(`Upload asset ${name}: bytes or filePath required`); + } + + const assets = existingAssets ?? (await getReleaseById(id, token)).assets ?? []; + const prior = assets.find((asset) => asset.name === name); + + /** @type {Uint8Array | null} */ + let priorBytes = null; if (prior) { - log(`[github-release] Replacing existing asset ${fileName} on release ${releaseId}`); - await deleteReleaseAsset(prior.id, token); + priorBytes = await downloadAsset(prior.id, token); + log(`[github-release] Replacing existing asset ${name} on release ${id}`); + await deleteAsset(prior.id, token); + } + + try { + if (usePath) { + return uploadFromFile(id, name, /** @type {string} */ (filePath), token, { + throwOnError: true, + }); + } + return await uploadBytes(id, name, /** @type {Uint8Array} */ (bytes), token, { + throwOnError: true, + }); + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + if (priorBytes) { + try { + await uploadBytes(id, name, priorBytes, token, { throwOnError: true }); + log(`[github-release] Restored prior asset ${name} on release ${id} after upload failure`); + } catch (restoreError) { + const restoreDetail = + restoreError instanceof Error ? restoreError.message : String(restoreError); + // Failure point: replacement upload failed and restore failed — prior asset gone. + fail( + `Upload asset ${name} to release ${id} failed (${detail}); restore of prior asset also failed (${restoreDetail})`, + ); + } + } + fail(`Upload asset ${name} to release ${id} failed: ${detail}`); } - return uploadReleaseAsset(releaseId, fileName, bytes, token); } /**