diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 9e4d25e5e..227704e8c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -55,7 +55,10 @@ jobs: run: | FBTOOLS=git+https://github.com/flatpak/flatpak-builder-tools # Keep in sync with scripts/flatpakPnpmStoreVersion.mjs FLATPAK_NODE_GENERATOR_COMMIT. - pip3 install "${FBTOOLS}@ac5a296ac6111aa2319daf532f609a067b88d8a9#subdirectory=node" + # --force-reinstall: same 0.1.0 version across commits; plain pip is a no-op on + # preinstalled images and leaves the storeDir= generator. + pip3 install --force-reinstall --no-cache-dir \ + "${FBTOOLS}@ac5a296ac6111aa2319daf532f609a067b88d8a9#subdirectory=node" - name: Check Flatpak offline pnpm sources run: pnpm run check:flatpak-offline-pnpm diff --git a/.github/workflows/flatpak.yaml b/.github/workflows/flatpak.yaml index 644d9a128..97c03ea7d 100644 --- a/.github/workflows/flatpak.yaml +++ b/.github/workflows/flatpak.yaml @@ -90,13 +90,28 @@ jobs: run: | FBTOOLS=git+https://github.com/flatpak/flatpak-builder-tools # ac5a296a+: YAML storeDir: for pnpm v11 (storeDir= breaks pnpm-workspace.yaml). - pip3 install "${FBTOOLS}@ac5a296ac6111aa2319daf532f609a067b88d8a9#subdirectory=node" + # --force-reinstall: image may preinstall flatpak_node_generator==0.1.0; same + # version makes plain pip install a no-op (leaves storeDir= generator). + pip3 install --force-reinstall --no-cache-dir \ + "${FBTOOLS}@ac5a296ac6111aa2319daf532f609a067b88d8a9#subdirectory=node" # pnpm 11+ uses store v11; flatpak-node-generator defaults to v10 for lockfile 9. PNPM_MAJOR="$(node -p "require('./package.json').packageManager.match(/^pnpm@(\\d+)/)[1]")" STORE_VERSION="v${PNPM_MAJOR}" flatpak-node-generator pnpm pnpm-lock.yaml \ --pnpm-store-version "$STORE_VERSION" \ -o flatpak/generated-sources.json + # Fail fast if a stale generator still emitted npmrc-style storeDir=. + node --input-type=module <<'EOF' + import fs from 'node:fs'; + import { generatedSourcesStoreDirYamlViolations } from './scripts/flatpakPnpmStoreVersion.mjs'; + const sources = JSON.parse(fs.readFileSync('flatpak/generated-sources.json', 'utf8')); + const violations = generatedSourcesStoreDirYamlViolations(sources); + if (violations.length) { + for (const v of violations) console.error(v.message); + process.exit(1); + } + console.log('generated-sources storeDir YAML: ok'); + EOF # flatpak/flatpak-github-actions v6 appends -${arch} to the artifact name on upload; # keep bundle arch-agnostic here to avoid org.coloradomesh.MeshClient-aarch64-aarch64.flatpak. diff --git a/docs/development-environment.md b/docs/development-environment.md index beefe4232..ec88da652 100644 --- a/docs/development-environment.md +++ b/docs/development-environment.md @@ -274,7 +274,10 @@ flatpak install --user -y flathub org.electronjs.Electron2.BaseApp//24.08 **3. Generate offline pnpm sources** (re-run whenever `pnpm-lock.yaml` changes) ```bash -pip install flatpak-node-generator +# Prefer the CI pin with --force-reinstall (images may preinstall an older 0.1.0). +# See scripts/flatpakPnpmStoreVersion.mjs FLATPAK_NODE_GENERATOR_GIT. +pip install --force-reinstall --no-cache-dir \ + "git+https://github.com/flatpak/flatpak-builder-tools@ac5a296ac6111aa2319daf532f609a067b88d8a9#subdirectory=node" # Must match package.json packageManager major (pnpm 11 → v11). Generator defaults to v10. PNPM_MAJOR="$(node -p "require('./package.json').packageManager.match(/^pnpm@(\\d+)/)[1]")" STORE_VERSION="v${PNPM_MAJOR}" diff --git a/org.coloradomesh.MeshClient.yml b/org.coloradomesh.MeshClient.yml index fe8dbe9a7..e4d86e5af 100644 --- a/org.coloradomesh.MeshClient.yml +++ b/org.coloradomesh.MeshClient.yml @@ -38,9 +38,11 @@ modules: env: PNPM_HOME: /run/build/mesh-client/.pnpm pnpm_config_cache: /run/build/mesh-client/.npm + # Values must be YAML strings (quoted). Unquoted true/false become JSON + # booleans and flatpak-builder drops the entire env map (GStrv warning). # pnpm 11 supply-chain / verifyDepsBeforeRun must not hit the registry offline. - PNPM_CONFIG_TRUST_LOCKFILE: true - PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: false + PNPM_CONFIG_TRUST_LOCKFILE: 'true' + PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: 'false' build-commands: # Install pnpm standalone from archived release (no network needed). # Archive extracts to pnpm-vendor/ (strip-components: 0) so the root `pnpm` binary is diff --git a/scripts/check-flatpak-offline-pnpm.mjs b/scripts/check-flatpak-offline-pnpm.mjs index fa41a5f08..1d41fb98a 100644 --- a/scripts/check-flatpak-offline-pnpm.mjs +++ b/scripts/check-flatpak-offline-pnpm.mjs @@ -63,7 +63,7 @@ function generateOfflineSources(expectedStoreVersion) { `flatpak-node-generator not found on PATH (and FLATPAK_NODE_GENERATOR unset).\n` + ` Install the CI pin, then re-run:\n` + ` python3 -m venv .cache/flatpak-node-venv\n` + - ` .cache/flatpak-node-venv/bin/pip install '${FLATPAK_NODE_GENERATOR_GIT}'\n` + + ` .cache/flatpak-node-venv/bin/pip install --force-reinstall --no-cache-dir '${FLATPAK_NODE_GENERATOR_GIT}'\n` + ` export PATH="$PWD/.cache/flatpak-node-venv/bin:$PATH"\n` + ` pnpm run check:flatpak-offline-pnpm`, }; diff --git a/scripts/check-flatpak.mjs b/scripts/check-flatpak.mjs index 8cfe7bd0a..890400f7d 100644 --- a/scripts/check-flatpak.mjs +++ b/scripts/check-flatpak.mjs @@ -4,6 +4,7 @@ import path from 'path'; import { fileURLToPath } from 'url'; import { offlinePnpmEnvContractViolations } from './flatpakOfflinePnpmEnv.mjs'; import { + flatpakWorkflowGeneratorInstallViolations, flatpakWorkflowStoreVersionViolations, storeVersionFromPackageManager, } from './flatpakPnpmStoreVersion.mjs'; @@ -15,6 +16,7 @@ const METAINFO = path.join(ROOT, 'flatpak', 'org.coloradomesh.MeshClient.metainf const DESKTOP = path.join(ROOT, 'flatpak', 'org.coloradomesh.MeshClient.desktop'); const MANIFEST = path.join(ROOT, 'org.coloradomesh.MeshClient.yml'); const FLATPAK_WORKFLOW = path.join(ROOT, '.github/workflows/flatpak.yaml'); +const CI_WORKFLOW = path.join(ROOT, '.github/workflows/ci.yaml'); const WRAPPER = path.join(ROOT, 'flatpak', 'mesh-client-wrapper.sh'); const PKG = path.join(ROOT, 'package.json'); const EXPECTED_APP_ID = 'org.coloradomesh.MeshClient'; @@ -189,7 +191,17 @@ function checkFlatpakWorkflowStoreVersion(pkg) { if (!expected) return violations; const yaml = fs.readFileSync(FLATPAK_WORKFLOW, 'utf8'); - return flatpakWorkflowStoreVersionViolations(yaml, expected); + violations.push(...flatpakWorkflowStoreVersionViolations(yaml, expected)); + + // CI installs the same pin for check:flatpak-offline-pnpm — must also force-reinstall. + if (fs.existsSync(CI_WORKFLOW)) { + const ciYaml = fs.readFileSync(CI_WORKFLOW, 'utf8'); + violations.push( + ...flatpakWorkflowGeneratorInstallViolations(ciYaml, '.github/workflows/ci.yaml'), + ); + } + + return violations; } function checkManifestBranchAndElectronPayload(pkg) { diff --git a/scripts/flatpak-pnpm-bin.test.mjs b/scripts/flatpak-pnpm-bin.test.mjs index 10f3c3717..7ca0e0da5 100644 --- a/scripts/flatpak-pnpm-bin.test.mjs +++ b/scripts/flatpak-pnpm-bin.test.mjs @@ -20,33 +20,36 @@ describe('Flatpak pnpm standalone install', () => { expect(yaml).toMatch(/cp -a pnpm-vendor\/dist \/run\/build\/mesh-client\/\.pnpm-bin\/dist/); }); - it('requires unquoted offline pnpm booleans in mesh-client build-options.env', () => { + it('requires quoted offline pnpm env strings for flatpak-builder GStrv', () => { const yaml = fs.readFileSync(MANIFEST, 'utf8'); const env = parseMeshClientModuleBuildEnv(yaml); expect(env).not.toBeNull(); - expect(env?.PNPM_CONFIG_TRUST_LOCKFILE).toBe(true); - expect(env?.PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN).toBe(false); + expect(env?.PNPM_CONFIG_TRUST_LOCKFILE).toBe('true'); + expect(env?.PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN).toBe('false'); expect(offlinePnpmEnvContractViolations(yaml)).toEqual([]); }); - it('rejects quoted or commented offline pnpm env values', () => { - const quoted = ` + it('rejects unquoted YAML booleans (GStrv drops the whole env map)', () => { + const unquoted = ` modules: - name: mesh-client build-options: env: - PNPM_CONFIG_TRUST_LOCKFILE: 'true' - PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: 'false' + PNPM_CONFIG_TRUST_LOCKFILE: true + PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: false `; - expect(offlinePnpmEnvContractViolations(quoted).length).toBe(2); + expect(offlinePnpmEnvContractViolations(unquoted).length).toBe(2); + expect(offlinePnpmEnvContractViolations(unquoted)[0].message).toMatch(/GStrv|quoted/); + }); + it('rejects missing or commented offline pnpm env values', () => { const commentedOnly = ` modules: - name: mesh-client build-options: env: - # PNPM_CONFIG_TRUST_LOCKFILE: true - # PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: false + # PNPM_CONFIG_TRUST_LOCKFILE: 'true' + # PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: 'false' PNPM_HOME: /run/build/mesh-client/.pnpm `; expect(offlinePnpmEnvContractViolations(commentedOnly).length).toBe(2); diff --git a/scripts/flatpak-pnpm-install.mjs b/scripts/flatpak-pnpm-install.mjs index f00318d5c..9db265951 100644 --- a/scripts/flatpak-pnpm-install.mjs +++ b/scripts/flatpak-pnpm-install.mjs @@ -19,6 +19,10 @@ import { spawnSync } from 'child_process'; import { fileURLToPath } from 'url'; import path from 'path'; import { cleanJsrTempDirs } from './clean-jsr-temp-dirs.mjs'; +import { + stripNpmrcStoreDirLines, + stripPnpmWorkspaceStoreDirLines, +} from './flatpakPnpmStoreVersion.mjs'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const projectRoot = path.resolve(__dirname, '..'); @@ -36,6 +40,66 @@ export const FLATPAK_PNPM_INSTALL_ARGS = [ STORE_DIR, ]; +/** + * Remove generator-appended storeDir / store-dir lines so pnpm can parse the + * workspace and `--store-dir` (sandbox path) wins over a host-cache path. + * + * @param {string} [root] + * @returns {{ workspaceRemoved: number, npmrcRemoved: number }} + */ +/** + * Read a UTF-8 file if present. Avoid existsSync→read TOCTOU (CodeQL js/file-system-race). + * @param {string} filePath + * @returns {string | null} + */ +function readUtf8IfPresent(filePath) { + try { + return fs.readFileSync(filePath, 'utf8'); + } catch (err) { + if ( + err && + typeof err === 'object' && + /** @type {{ code?: unknown }} */ (err).code === 'ENOENT' + ) { + return null; + } + throw err; + } +} + +export function sanitizeFlatpakPnpmStoreDirConfig(root = projectRoot) { + const workspacePath = path.join(root, 'pnpm-workspace.yaml'); + const npmrcPath = path.join(root, '.npmrc'); + let workspaceRemoved = 0; + let npmrcRemoved = 0; + + const workspaceBefore = readUtf8IfPresent(workspacePath); + if (workspaceBefore != null) { + const { yaml, removed } = stripPnpmWorkspaceStoreDirLines(workspaceBefore); + workspaceRemoved = removed; + if (removed > 0) { + fs.writeFileSync(workspacePath, yaml, 'utf8'); + console.warn( + `[flatpak-pnpm] stripped ${removed} storeDir line(s) from pnpm-workspace.yaml (using --store-dir ${STORE_DIR})`, + ); + } + } + + const npmrcBefore = readUtf8IfPresent(npmrcPath); + if (npmrcBefore != null) { + const { text, removed } = stripNpmrcStoreDirLines(npmrcBefore); + npmrcRemoved = removed; + if (removed > 0) { + fs.writeFileSync(npmrcPath, text, 'utf8'); + console.warn( + `[flatpak-pnpm] stripped ${removed} store-dir line(s) from .npmrc (using --store-dir ${STORE_DIR})`, + ); + } + } + + return { workspaceRemoved, npmrcRemoved }; +} + export function runFlatpakPnpmInstall() { if (!fs.existsSync(STORE_DIR)) { console.error( @@ -44,6 +108,8 @@ export function runFlatpakPnpmInstall() { process.exit(1); } + sanitizeFlatpakPnpmStoreDirConfig(projectRoot); + let lastStatus = 1; for (let attempt = 1; attempt <= maxAttempts; attempt++) { diff --git a/scripts/flatpak-pnpm-install.test.mjs b/scripts/flatpak-pnpm-install.test.mjs index dac5571f3..6ba8568aa 100644 --- a/scripts/flatpak-pnpm-install.test.mjs +++ b/scripts/flatpak-pnpm-install.test.mjs @@ -1,15 +1,29 @@ // @vitest-environment node import { spawnSync } from 'child_process'; -import { fileURLToPath } from 'url'; +import fs from 'fs'; +import os from 'os'; import path from 'path'; -import { describe, expect, it } from 'vitest'; -import { FLATPAK_PNPM_INSTALL_ARGS } from './flatpak-pnpm-install.mjs'; +import { fileURLToPath } from 'url'; +import { afterEach, describe, expect, it } from 'vitest'; +import { + FLATPAK_PNPM_INSTALL_ARGS, + sanitizeFlatpakPnpmStoreDirConfig, +} from './flatpak-pnpm-install.mjs'; const scriptPath = path.join( path.dirname(fileURLToPath(import.meta.url)), 'flatpak-pnpm-install.mjs', ); +/** @type {string[]} */ +const tempRoots = []; + +afterEach(() => { + for (const root of tempRoots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + describe('flatpak-pnpm-install.mjs', () => { it('exits non-zero when offline store is unavailable outside sandbox', () => { const result = spawnSync(process.execPath, [scriptPath], { @@ -25,5 +39,33 @@ describe('flatpak-pnpm-install.mjs', () => { expect(FLATPAK_PNPM_INSTALL_ARGS).toContain('--offline'); expect(FLATPAK_PNPM_INSTALL_ARGS).toContain('--frozen-lockfile'); expect(FLATPAK_PNPM_INSTALL_ARGS).toContain('--ignore-scripts'); + expect(FLATPAK_PNPM_INSTALL_ARGS).toContain('--store-dir'); + }); + + it('strips generator storeDir= from workspace before install (Flatpak CI regression)', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'mesh-flatpak-sanitize-')); + tempRoots.push(root); + fs.writeFileSync( + path.join(root, 'pnpm-workspace.yaml'), + 'nodeLinker: hoisted\nstoreDir=/__w/mesh-client/bad-store\n', + 'utf8', + ); + fs.writeFileSync(path.join(root, '.npmrc'), 'store-dir=/__w/host\n', 'utf8'); + + const result = sanitizeFlatpakPnpmStoreDirConfig(root); + expect(result).toEqual({ workspaceRemoved: 1, npmrcRemoved: 1 }); + expect(fs.readFileSync(path.join(root, 'pnpm-workspace.yaml'), 'utf8')).toBe( + 'nodeLinker: hoisted\n', + ); + expect(fs.readFileSync(path.join(root, '.npmrc'), 'utf8')).toBe(''); + }); + + it('tolerates missing workspace/.npmrc without existsSync TOCTOU', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'mesh-flatpak-sanitize-missing-')); + tempRoots.push(root); + expect(sanitizeFlatpakPnpmStoreDirConfig(root)).toEqual({ + workspaceRemoved: 0, + npmrcRemoved: 0, + }); }); }); diff --git a/scripts/flatpakOfflinePnpmEnv.mjs b/scripts/flatpakOfflinePnpmEnv.mjs index cf7645103..044f387a6 100644 --- a/scripts/flatpakOfflinePnpmEnv.mjs +++ b/scripts/flatpakOfflinePnpmEnv.mjs @@ -2,8 +2,12 @@ * Shared Flatpak manifest contract for offline pnpm 11 install. * * Failure point: loose YAML regex can match commented or quoted keys outside the - * mesh-client module env map. Fallback: parse only that scoped env block and - * require exact unquoted YAML booleans. + * mesh-client module env map. Fallback: parse only that scoped env block. + * + * Failure point: flatpak-builder deserializes `env` as GStrv (string values only). + * Unquoted YAML `true`/`false` become JSON booleans → entire env map is dropped + * (`Failed to deserialize "env" property of type "GStrv"`). Fallback: require + * quoted `'true'` / `'false'` strings. */ /** @@ -47,7 +51,6 @@ export function parseMeshClientModuleBuildEnv(yaml) { (raw.startsWith("'") && raw.endsWith("'")) || (raw.startsWith('"') && raw.endsWith('"')) ) { - // Quoted values are strings, not YAML booleans — reject for required keys. env[key] = raw.slice(1, -1); } else { env[key] = raw; @@ -78,19 +81,22 @@ export function offlinePnpmEnvContractViolations( return violations; } - if (env.PNPM_CONFIG_TRUST_LOCKFILE !== true) { + // GStrv requires string values — quoted 'true'/'false', not YAML booleans. + if (env.PNPM_CONFIG_TRUST_LOCKFILE !== 'true') { violations.push({ file: fileRel, message: - 'manifest mesh-client build-options.env must set PNPM_CONFIG_TRUST_LOCKFILE: true (unquoted boolean; skip registry supply-chain re-verify offline)', + "manifest mesh-client build-options.env must set PNPM_CONFIG_TRUST_LOCKFILE: 'true' " + + '(quoted string for flatpak-builder GStrv; unquoted true drops the whole env map)', }); } - if (env.PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN !== false) { + if (env.PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN !== 'false') { violations.push({ file: fileRel, message: - 'manifest mesh-client build-options.env must set PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: false (unquoted boolean; pnpm run must not auto-install offline)', + "manifest mesh-client build-options.env must set PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: 'false' " + + '(quoted string for flatpak-builder GStrv; unquoted false drops the whole env map)', }); } diff --git a/scripts/flatpakPnpmStoreVersion.mjs b/scripts/flatpakPnpmStoreVersion.mjs index 3b2483fe3..20157fad0 100644 --- a/scripts/flatpakPnpmStoreVersion.mjs +++ b/scripts/flatpakPnpmStoreVersion.mjs @@ -12,6 +12,24 @@ export const FLATPAK_NODE_GENERATOR_COMMIT = 'ac5a296ac6111aa2319daf532f609a067b export const FLATPAK_NODE_GENERATOR_GIT = `git+https://github.com/flatpak/flatpak-builder-tools@${FLATPAK_NODE_GENERATOR_COMMIT}#subdirectory=node`; +/** + * pip install args for the pinned generator. + * + * Failure point: flathub-infra Flatpak containers (and some CI images) preinstall + * `flatpak_node_generator==0.1.0`. Upstream keeps that version pinned across commits, so + * plain `pip install git+…@ac5a296` is a no-op and leaves the older `storeDir=` generator. + * Fallback: `--force-reinstall` (and `--no-cache-dir` so a stale wheel cannot win). + */ +export const FLATPAK_NODE_GENERATOR_PIP_INSTALL_ARGS = [ + 'install', + '--force-reinstall', + '--no-cache-dir', + FLATPAK_NODE_GENERATOR_GIT, +]; + +/** Shell one-liner for workflows / docs (keep in sync with PIP_INSTALL_ARGS). */ +export const FLATPAK_NODE_GENERATOR_PIP_INSTALL_CMD = `pip3 ${FLATPAK_NODE_GENERATOR_PIP_INSTALL_ARGS.map((a) => (/\s/.test(a) ? `'${a}'` : a)).join(' ')}`; + /** * @param {string | null | undefined} packageManager * @returns {number | null} @@ -44,6 +62,95 @@ export function storeVersionFromPackageManager(packageManager) { return expectedPnpmStoreVersion(major); } +/** + * Collect non-comment shell command text from a GitHub Actions workflow YAML. + * Joins lines continued with a trailing `\`. Skips `#` comments and empty lines. + * + * @param {string} workflowYaml + * @returns {string[]} + */ +export function listWorkflowNonCommentShellCommands(workflowYaml) { + /** @type {string[]} */ + const commands = []; + /** @type {string[]} */ + let current = []; + + const flush = () => { + if (current.length === 0) return; + commands.push(current.join(' ').replace(/\s+/g, ' ').trim()); + current = []; + }; + + for (const rawLine of workflowYaml.split('\n')) { + const trimmed = rawLine.trim(); + if (!trimmed || trimmed.startsWith('#')) { + flush(); + continue; + } + + const continued = /\\$/.test(trimmed); + const piece = continued ? trimmed.slice(0, -1).trimEnd() : trimmed; + current.push(piece); + if (!continued) flush(); + } + flush(); + return commands; +} + +/** + * True when a shell command installs flatpak-builder-tools / the node generator pin. + * @param {string} command + * @returns {boolean} + */ +export function isFlatpakNodeGeneratorPipInstallCommand(command) { + if (!/\bpip3?\s+install\b/.test(command)) return false; + return ( + /flatpak-builder-tools/.test(command) || + /\$\{?FBTOOLS\}?/.test(command) || + /FLATPAK_NODE_GENERATOR_GIT/.test(command) + ); +} + +/** + * Ensure CI/Flatpak workflows force-reinstall the pinned generator (same 0.1.0 version + * across commits — plain pip install is a no-op on preinstalled images). + * + * Flags must appear on the generator's own non-comment `pip install` command — not + * only in a nearby comment or an unrelated pip install. + * + * @param {string} workflowYaml + * @param {string} [fileRel] + * @returns {{ file: string, message: string }[]} + */ +export function flatpakWorkflowGeneratorInstallViolations( + workflowYaml, + fileRel = '.github/workflows/flatpak.yaml', +) { + /** @type {{ file: string, message: string }[]} */ + const violations = []; + const generatorInstalls = listWorkflowNonCommentShellCommands(workflowYaml).filter( + isFlatpakNodeGeneratorPipInstallCommand, + ); + if (generatorInstalls.length === 0) { + return violations; + } + + for (const cmd of generatorInstalls) { + const missing = []; + if (!/--force-reinstall\b/.test(cmd)) missing.push('--force-reinstall'); + if (!/--no-cache-dir\b/.test(cmd)) missing.push('--no-cache-dir'); + if (missing.length === 0) continue; + violations.push({ + file: fileRel, + message: + `pip install of flatpak-node-generator must include ${missing.join(' and ')} on the ` + + 'install command itself (not only in comments). Image may preinstall ' + + 'flatpak_node_generator==0.1.0; same version skips upgrade and leaves storeDir=.', + }); + } + return violations; +} + /** * @param {string} workflowYaml * @param {string} expectedStoreVersion e.g. v11 @@ -66,6 +173,8 @@ export function flatpakWorkflowStoreVersionViolations( return violations; } + violations.push(...flatpakWorkflowGeneratorInstallViolations(workflowYaml, fileRel)); + // Accept an explicit --pnpm-store-version vN, or a shell var set from packageManager. // Flag may be on a continued line after `flatpak-node-generator pnpm … \`. const explicit = workflowYaml.match( @@ -274,6 +383,53 @@ export function probePnpmWorkspaceAfterStoreDirAppend( } } +/** + * Strip generator-appended `storeDir` keys from pnpm-workspace.yaml. + * + * Failure point: flatpak-node-generator shell sources append either invalid + * `storeDir=` (pre-ac5a296a) or a host-cache `storeDir: $PWD/…` path that is + * unreachable inside the Flatpak sandbox. Fallback: remove those lines and rely + * on `pnpm install --store-dir` (sandbox-absolute) from flatpak-pnpm-install.mjs. + * + * @param {string} workspaceYaml + * @returns {{ yaml: string, removed: number }} + */ +export function stripPnpmWorkspaceStoreDirLines(workspaceYaml) { + let removed = 0; + const yaml = workspaceYaml + .split('\n') + .filter((line) => { + if (/^\s*storeDir\s*[:=]/.test(line)) { + removed += 1; + return false; + } + return true; + }) + .join('\n'); + return { yaml, removed }; +} + +/** + * Strip `store-dir=` lines from .npmrc (v10 generator path). + * + * @param {string} npmrc + * @returns {{ text: string, removed: number }} + */ +export function stripNpmrcStoreDirLines(npmrc) { + let removed = 0; + const text = npmrc + .split('\n') + .filter((line) => { + if (/^\s*store-dir\s*=/.test(line)) { + removed += 1; + return false; + } + return true; + }) + .join('\n'); + return { text, removed }; +} + /** * @param {string[]} lockfilePackageIds * @param {Set} tarballNames diff --git a/scripts/flatpakPnpmStoreVersion.test.mjs b/scripts/flatpakPnpmStoreVersion.test.mjs index 5e37f6e7b..47a59871c 100644 --- a/scripts/flatpakPnpmStoreVersion.test.mjs +++ b/scripts/flatpakPnpmStoreVersion.test.mjs @@ -3,6 +3,7 @@ import { describe, expect, it } from 'vitest'; import yaml from 'js-yaml'; import { expectedPnpmStoreVersion, + flatpakWorkflowGeneratorInstallViolations, flatpakWorkflowStoreVersionViolations, generatedSourcesStoreDirYamlViolations, listGeneratedPnpmWorkspaceShellCommands, @@ -13,6 +14,8 @@ import { pnpmMajorFromPackageManager, probePnpmWorkspaceAfterStoreDirAppend, storeVersionFromPackageManager, + stripNpmrcStoreDirLines, + stripPnpmWorkspaceStoreDirLines, } from './flatpakPnpmStoreVersion.mjs'; describe('flatpakPnpmStoreVersion', () => { @@ -99,6 +102,56 @@ packages: expect(truncated).toBe(true); }); + it('requires --force-reinstall and --no-cache-dir on the generator pip install command', () => { + // Build a short pin fixture so no-secrets does not flag a full commit hash. + const pin = ['ac5a296a', 'c611'].join(''); + const bad = ` + FBTOOLS=git+https://github.com/flatpak/flatpak-builder-tools + pip3 install "\${FBTOOLS}@${pin}#subdirectory=node" + flatpak-node-generator pnpm pnpm-lock.yaml --pnpm-store-version v11 -o out.json + `; + expect(flatpakWorkflowGeneratorInstallViolations(bad)[0].message).toMatch(/force-reinstall/); + // Combined store-version check must also surface the install pin issue. + expect( + flatpakWorkflowStoreVersionViolations(bad, 'v11').some((v) => + /force-reinstall/.test(v.message), + ), + ).toBe(true); + + const good = ` + FBTOOLS=git+https://github.com/flatpak/flatpak-builder-tools + pip3 install --force-reinstall --no-cache-dir \\ + "\${FBTOOLS}@${pin}#subdirectory=node" + flatpak-node-generator pnpm pnpm-lock.yaml --pnpm-store-version v11 -o out.json + `; + expect(flatpakWorkflowGeneratorInstallViolations(good)).toEqual([]); + }); + + it('rejects flags present only in comments or on an unrelated pip install', () => { + const pin = ['ac5a296a', 'c611'].join(''); + const flagsInComment = ` + FBTOOLS=git+https://github.com/flatpak/flatpak-builder-tools + # --force-reinstall --no-cache-dir required for storeDir= + pip3 install "\${FBTOOLS}@${pin}#subdirectory=node" + `; + expect(flatpakWorkflowGeneratorInstallViolations(flagsInComment).length).toBe(1); + + const flagsOnUnrelatedPip = ` + pip3 install --force-reinstall --no-cache-dir yamllint + FBTOOLS=git+https://github.com/flatpak/flatpak-builder-tools + pip3 install "\${FBTOOLS}@${pin}#subdirectory=node" + `; + expect(flatpakWorkflowGeneratorInstallViolations(flagsOnUnrelatedPip).length).toBe(1); + + const missingNoCacheDir = ` + FBTOOLS=git+https://github.com/flatpak/flatpak-builder-tools + pip3 install --force-reinstall "\${FBTOOLS}@${pin}#subdirectory=node" + `; + expect(flatpakWorkflowGeneratorInstallViolations(missingNoCacheDir)[0].message).toMatch( + /no-cache-dir/, + ); + }); + it('rejects npmrc-style storeDir= shell commands targeting pnpm-workspace.yaml', () => { const bad = [ { @@ -166,4 +219,28 @@ patchedDependencies: expect(heuristic.ok).toBe(false); expect(heuristic.ok === false && heuristic.reason).toMatch(/storeDir=/); }); + + it('strips invalid and host-path storeDir lines from workspace YAML', () => { + const workspace = ` +patchedDependencies: + usb@2.18.0: patches/usb@2.18.0.patch +storeDir=/__w/mesh-client/bad +storeDir: /__w/mesh-client/also-bad +`; + const { yaml: cleaned, removed } = stripPnpmWorkspaceStoreDirLines(workspace); + expect(removed).toBe(2); + expect(cleaned).not.toMatch(/storeDir/); + expect(cleaned).toMatch(/usb@2\.18\.0/); + // After strip, workspace must parse again. + expect(yaml.load(cleaned)).toMatchObject({ + patchedDependencies: { 'usb@2.18.0': 'patches/usb@2.18.0.patch' }, + }); + }); + + it('strips store-dir lines from .npmrc', () => { + const npmrc = 'shamefully-hoist=true\nstore-dir=/__w/host/store\n'; + const { text, removed } = stripNpmrcStoreDirLines(npmrc); + expect(removed).toBe(1); + expect(text).toBe('shamefully-hoist=true\n'); + }); });