chore: keep the fetch-deps archive out of the checkout

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);
   }
 }

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<string, string>)[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);
+  });
+});