From bb4bd1b70bcc3b87fe2da020a7eb873fd9436933 Mon Sep 17 00:00:00 2001 From: Luis Rivas Date: Fri, 13 Nov 2026 11:56:06 -0500 Subject: [PATCH] Keep the fetch-deps archive out of the checkout Downloads go to a temporary directory under the OS temp dir, removed on success and failure. A local archive is extracted in place and never deleted; one inside deps/ is refused, since deps/ is cleared first. --- scripts/fetch-deps.js | 57 ++++++----- test/fetch_deps.test.ts | 213 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 243 insertions(+), 27 deletions(-) create mode 100644 test/fetch_deps.test.ts diff --git a/scripts/fetch-deps.js b/scripts/fetch-deps.js index 2304670..b6e754c 100644 --- a/scripts/fetch-deps.js +++ b/scripts/fetch-deps.js @@ -3,8 +3,9 @@ 'use strict'; const fs = require('fs'); +const os = require('os'); const path = require('path'); -const { execSync, execFileSync } = require('child_process'); +const { execFileSync } = require('child_process'); const ROOT = path.join(__dirname, '..'); const DEPS_DIR = path.join(ROOT, 'deps'); @@ -62,7 +63,6 @@ const repo = process.env.MAVEN_REPO || 'releases'; // same host the token was issued for. const baseUrl = (process.env.ARTIFACTORY_URL || 'https://twilio.jfrog.io').replace(/\/+$/, ''); const repoUrl = `${baseUrl}/artifactory/${repo}`; -const tmpFile = path.join(ROOT, '.tmp-twilio-video.tar.bz2'); // Runs one fetch attempt up to RETRIES times. Returns the error from the final // attempt, so each caller decides how much of it is safe to surface. @@ -83,8 +83,7 @@ function withRetries(attemptFetch) { // Fetch with an Artifactory access token instead of Maven credentials. Separate // from npm's token so it can be scoped to this artifact's repository. CI mints a // short-lived one via OIDC; locally a personal token works. -function curlGet(token) { - const version = resolveVersion(); +function curlGet(token, version, dest) { // The artifact the Maven coordinates in mvnGet resolve to, addressed directly. const downloadUrl = `${repoUrl}/com/twilio/sdk/twilio-video/${version}/twilio-video-${version}-${platform}.tar.bz2`; log(`Fetching twilio-video from Artifactory`); @@ -96,18 +95,7 @@ function curlGet(token) { // and the retries below never get a turn. execFileSync( 'curl', - [ - '-fsSL', - '--connect-timeout', - '30', - '--max-time', - '900', - '-K', - '-', - '-o', - tmpFile, - downloadUrl, - ], + ['-fsSL', '--connect-timeout', '30', '--max-time', '900', '-K', '-', '-o', dest, downloadUrl], { input: `header = "Authorization: Bearer ${token}"\n`, stdio: ['pipe', 'inherit', 'inherit'], @@ -124,8 +112,8 @@ function curlGet(token) { ); } -function mvnGet() { - const artifact = `com.twilio.sdk:twilio-video:${resolveVersion()}:tar.bz2:${platform}`; +function mvnGet(version, dest) { + const artifact = `com.twilio.sdk:twilio-video:${version}:tar.bz2:${platform}`; log(`Fetching twilio-video via Maven`); log(` artifact: ${artifact}`); log(` repo: ${repoUrl}`); @@ -140,7 +128,7 @@ function mvnGet() { `-DrepoUrl=${repoUrl}`, '-Dtransitive=false', `-Dartifact=${artifact}`, - `-Ddest=${tmpFile}`, + `-Ddest=${dest}`, ], { stdio: 'inherit' }, ), @@ -156,25 +144,40 @@ function mvnGet() { } function main() { + let tmpDir; try { const args = process.argv.slice(2); const pkgIndex = args.indexOf('--twilio-video-pkg'); const localArchive = (pkgIndex !== -1 ? args[pkgIndex + 1] : null) || process.env.RTC_CPP_ARCHIVE; + let archive; if (localArchive) { const resolved = path.resolve(localArchive); if (!fs.existsSync(resolved)) { throw new Error(`Local archive not found: ${resolved}`); } + // deps/ is cleared before extraction, which would delete the archive. The + // native realpath also normalizes case on case-insensitive volumes. + const depsReal = fs.existsSync(DEPS_DIR) ? fs.realpathSync.native(DEPS_DIR) : DEPS_DIR; + if (fs.realpathSync.native(resolved).startsWith(depsReal + path.sep)) { + throw new Error(`Local archive must be outside deps/: ${resolved}`); + } log(`Using local archive: ${resolved}`); - fs.copyFileSync(resolved, tmpFile); - } else if (process.env.RTC_CPP_ARTIFACTORY_TOKEN) { - curlGet(process.env.RTC_CPP_ARTIFACTORY_TOKEN); + archive = resolved; } else { - mvnGet(); + const version = resolveVersion(); + // Outside the checkout: docker/run.sh bind-mounts it, so a download there + // is written through to the host. + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'twilio-video-')); + archive = path.join(tmpDir, 'twilio-video.tar.bz2'); + if (process.env.RTC_CPP_ARTIFACTORY_TOKEN) { + curlGet(process.env.RTC_CPP_ARTIFACTORY_TOKEN, version, archive); + } else { + mvnGet(version, archive); + } } - const size = fs.statSync(tmpFile).size; + const size = fs.statSync(archive).size; log(`Downloaded ${(size / 1024 / 1024).toFixed(1)} MB`); // docker/run.sh mounts a volume at deps/, which can't be removed @@ -190,9 +193,9 @@ function main() { fs.mkdirSync(DEPS_DIR, { recursive: true }); log('Extracting...'); - execSync(`tar xjf "${tmpFile}" -C "${DEPS_DIR}"`, { stdio: 'inherit' }); + execFileSync('tar', ['xjf', archive, '-C', DEPS_DIR], { stdio: 'inherit' }); - fs.unlinkSync(tmpFile); + if (tmpDir) fs.rmSync(tmpDir, { recursive: true }); // The Maven artifact extracts to deps/twilio-video/ which CMakeLists.txt expects const videoDir = path.join(DEPS_DIR, 'twilio-video'); @@ -248,7 +251,7 @@ function main() { log(`rtc-cpp artifacts ready at deps/twilio-video/ (${buildType})`); } catch (err) { console.error(`[fetch-deps] Failed: ${err.message}`); - if (fs.existsSync(tmpFile)) fs.unlinkSync(tmpFile); + if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); process.exit(1); } } diff --git a/test/fetch_deps.test.ts b/test/fetch_deps.test.ts new file mode 100644 index 0000000..a8109e2 --- /dev/null +++ b/test/fetch_deps.test.ts @@ -0,0 +1,213 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { spawnSync, execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +// The script uses its parent directory as the checkout, so each run copies it +// into a throwaway one. Shims stand in for curl and record tar's view of the run. +const archDir = ({ x64: 'x86_64', arm64: 'arm64' } as Record)[process.arch]; + +let work: string; +let validArchive: string; +let corruptArchive: string; +let shimDir: string; + +beforeAll(() => { + work = fs.mkdtempSync(path.join(os.tmpdir(), 'fetch-deps-test-')); + + // The smallest layout the script accepts after extraction. + const src = path.join(work, 'archive-src'); + const libDir = path.join(src, 'twilio-video', 'lib', archDir, 'appleclang-test', 'release'); + fs.mkdirSync(path.join(src, 'twilio-video', 'include'), { recursive: true }); + fs.mkdirSync(libDir, { recursive: true }); + for (const lib of ['libtwilio-video.a', 'libwebrtc.a', 'libjsoncpp.a']) { + fs.writeFileSync(path.join(libDir, lib), ''); + } + validArchive = path.join(work, 'valid.tar.bz2'); + execFileSync('tar', ['cjf', validArchive, '-C', src, 'twilio-video']); + + corruptArchive = path.join(work, 'corrupt.tar.bz2'); + fs.writeFileSync(corruptArchive, 'not a bzip2 stream'); + + const realTar = execFileSync('sh', ['-c', 'command -v tar'], { encoding: 'utf8' }).trim(); + shimDir = path.join(work, 'bin'); + fs.mkdirSync(shimDir); + fs.writeFileSync( + path.join(shimDir, 'tar'), + '#!/bin/sh\n' + + 'printf "%s\\n" "$@" > "$SHIM_OUT/args"\n' + + 'ls -A "$SHIM_ROOT" > "$SHIM_OUT/root"\n' + + `exec "${realTar}" "$@"\n`, + { mode: 0o755 }, + ); + // Serves CURL_SERVES as the download, written to the path after `-o`. + fs.writeFileSync( + path.join(shimDir, 'curl'), + '#!/bin/sh\n' + + 'while [ $# -gt 0 ]; do [ "$1" = "-o" ] && out="$2"; shift; done\n' + + 'cat > /dev/null\n' + + 'exec cp "$CURL_SERVES" "$out"\n', + { mode: 0o755 }, + ); +}); + +afterAll(() => { + fs.rmSync(work, { recursive: true, force: true }); +}); + +interface RunOptions { + /** Passed as RTC_CPP_ARCHIVE. */ + local?: string; + /** Served by the curl shim, with RTC_CPP_ARTIFACTORY_TOKEN set. */ + download?: string; + /** Copied into the checkout's deps/ and passed as RTC_CPP_ARCHIVE. */ + localInDeps?: string; + /** Spells deps/ as DEPS/ in RTC_CPP_ARCHIVE. */ + upperCaseDeps?: boolean; + pin?: boolean; + tmpDirExists?: boolean; +} + +/** Runs a copy of the script in a fresh checkout, with its own TMPDIR. */ +function run({ + local, + localInDeps, + upperCaseDeps = false, + download, + pin = true, + tmpDirExists = true, +}: RunOptions) { + const dir = fs.mkdtempSync(path.join(work, 'run-')); + const root = path.join(dir, 'checkout'); + const tmpDir = path.join(dir, 'tmp'); + const shimOut = path.join(dir, 'shim'); + fs.mkdirSync(path.join(root, 'scripts'), { recursive: true }); + if (tmpDirExists) fs.mkdirSync(tmpDir); + fs.mkdirSync(shimOut); + const script = path.join(root, 'scripts', 'fetch-deps.js'); + fs.copyFileSync(path.join(__dirname, '..', 'scripts', 'fetch-deps.js'), script); + if (pin) fs.writeFileSync(path.join(root, '.rtc-cpp-version'), '0.0.0\n'); + + const env: NodeJS.ProcessEnv = { + ...process.env, + PATH: `${shimDir}${path.delimiter}${process.env.PATH}`, + TMPDIR: tmpDir, + SHIM_OUT: shimOut, + SHIM_ROOT: root, + ARTIFACTORY_URL: 'http://127.0.0.1:1', + }; + // The developer's own settings would change which path the script takes. + for (const name of [ + 'RTC_CPP_ARCHIVE', + 'RTC_CPP_ARTIFACTORY_TOKEN', + 'RTC_CPP_VERSION', + 'RTC_CPP_BUILD_TYPE', + 'RTC_CPP_ARCH', + 'MAVEN_REPO', + ]) { + delete env[name]; + } + if (local) env.RTC_CPP_ARCHIVE = local; + if (localInDeps) { + fs.mkdirSync(path.join(root, 'deps')); + fs.copyFileSync(localInDeps, path.join(root, 'deps', 'archive.tar.bz2')); + env.RTC_CPP_ARCHIVE = path.join(root, upperCaseDeps ? 'DEPS' : 'deps', 'archive.tar.bz2'); + } + if (download) { + env.RTC_CPP_ARTIFACTORY_TOKEN = 'test-token'; + env.CURL_SERVES = download; + } + + const result = spawnSync(process.execPath, [script], { encoding: 'utf8', env }); + const read = (name: string) => fs.readFileSync(path.join(shimOut, name), 'utf8').split('\n'); + + return { + result, + tmpDir, + local: env.RTC_CPP_ARCHIVE, + tarArgs: () => read('args'), + rootAtExtract: () => read('root'), + }; +} + +describe('fetch-deps downloaded archive', () => { + it('extracts from the OS temp dir, with no archive in the checkout', () => { + const { result, tmpDir, tarArgs, rootAtExtract } = run({ download: validArchive }); + + expect(result.status, result.stderr).toBe(0); + const archive = tarArgs()[1]; + expect(archive.startsWith(tmpDir + path.sep), `archive at ${archive}`).toBe(true); + expect(rootAtExtract().filter(Boolean).sort()).toEqual(['.rtc-cpp-version', 'deps', 'scripts']); + }); + + it('is removed after a successful run', () => { + const { result, tmpDir } = run({ download: validArchive }); + + expect(result.status, result.stderr).toBe(0); + expect(fs.readdirSync(tmpDir)).toEqual([]); + }); + + it('is removed when extraction fails', () => { + const { result, tmpDir } = run({ download: corruptArchive }); + + expect(result.status).toBe(1); + expect(fs.readdirSync(tmpDir)).toEqual([]); + }); + + it('leaves nothing in the OS temp dir when the version pin is missing', () => { + const { result, tmpDir } = run({ download: validArchive, pin: false }); + + expect(result.status).toBe(1); + expect(fs.readdirSync(tmpDir)).toEqual([]); + }); + + it('reports a missing OS temp dir as a fetch-deps failure', () => { + const { result } = run({ download: validArchive, tmpDirExists: false }); + + expect(result.status).toBe(1); + expect(result.stderr).toMatch(/^\[fetch-deps\] Failed: .*ENOENT/m); + }); +}); + +describe('fetch-deps local archive', () => { + it('is extracted in place, without a copy', () => { + const { result, tarArgs } = run({ local: validArchive }); + + expect(result.status, result.stderr).toBe(0); + expect(tarArgs()[1]).toBe(validArchive); + }); + + it('is extracted when its path has shell metacharacters', () => { + const odd = path.join(work, 'a $HOME `x` "q".tar.bz2'); + fs.copyFileSync(validArchive, odd); + + const { result } = run({ local: odd }); + + expect(result.status, result.stderr).toBe(0); + }); + + it('is refused, and kept, when it is inside deps/', () => { + const { result, local } = run({ localInDeps: validArchive }); + + expect(result.status).toBe(1); + expect(result.stderr).toMatch(/^\[fetch-deps\] Failed: .*outside deps\//m); + expect(fs.existsSync(local!)).toBe(true); + }); + + // The default macOS volume is case-insensitive, so DEPS/ is deps/ there. + it.runIf(process.platform === 'darwin')('is refused when deps/ is spelled DEPS/', () => { + const { result, local } = run({ localInDeps: validArchive, upperCaseDeps: true }); + + expect(result.status).toBe(1); + expect(result.stderr).toMatch(/^\[fetch-deps\] Failed: .*outside deps\//m); + expect(fs.existsSync(local!)).toBe(true); + }); + + it('is kept when extraction fails', () => { + const { result } = run({ local: corruptArchive }); + + expect(result.status).toBe(1); + expect(fs.existsSync(corruptArchive)).toBe(true); + }); +});