fix(ios-prebuild): address review — enforce inventory collisions, harden compose freshness + shell usage

Review feedback on the header modularization (VFS-overlay removal):

- computeInventory() now returns natural-path collisions; computeSpecPlan()
  fails closed on them (R8) so two sources projecting to the same Headers/
  path can no longer be silently merged (only identities[0].source was kept).
- ensureHeadersLayout()'s freshness marker folds a sha256 of the compose
  tooling (headers-{inventory,spec,compose}.js) so a local script edit forces
  a recompose — the source xcframework's Info.plist mtime alone couldn't.
- scanHeader() strips /* ... */ block comments (multi-line aware) so a
  documentation line mentioning namespace/template/constexpr can't trip the
  C++ detector and shrink the umbrella.
- Anonymous aggregates with C++ member initializers (typedef struct { x = ..; })
  are now detected as ObjC++ (tag name made optional).
- Multi-hyphen pod names namespaced correctly (.replace(/-/g,'_')).
- cp/tar shell-outs switched to execFileSync with arg arrays (CodeQL
  "command from environment values"; parity with headers-verify.js).
- Clearer hermes-missing warning; headers-spec R9/R10 added to the contract
  docblock (was R1–R8 then R11).
- New headers-inventory-test.js covering scanHeader (cxx guards, #else/#elif
  flips, anonymous aggregate, block/line comments).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: chrfalch

packages/react-native/scripts/ios-prebuild/__tests__/headers-inventory-test.js

@@ -0,0 +1,118 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict-local
+ * @format
+ */
+
+'use strict';
+
+const {scanHeader} = require('../headers-inventory');
+
+describe('scanHeader include classification', () => {
+  test('an unguarded include is not cxx-guarded', () => {
+    const r = scanHeader('#import <React/RCTBridge.h>\n');
+    expect(r.includes).toEqual([
+      {token: 'React/RCTBridge.h', cxxGuarded: false},
+    ]);
+  });
+
+  test('an include under #ifdef __cplusplus is cxx-guarded', () => {
+    const r = scanHeader(
+      '#ifdef __cplusplus\n#include <folly/dynamic.h>\n#endif\n',
+    );
+    expect(r.includes).toEqual([{token: 'folly/dynamic.h', cxxGuarded: true}]);
+  });
+
+  test('#else flips the __cplusplus guard', () => {
+    const src = [
+      '#ifdef __cplusplus',
+      '#include <cpp/only.h>',
+      '#else',
+      '#include <c/only.h>',
+      '#endif',
+      '',
+    ].join('\n');
+    expect(scanHeader(src).includes).toEqual([
+      {token: 'cpp/only.h', cxxGuarded: true},
+      {token: 'c/only.h', cxxGuarded: false},
+    ]);
+  });
+
+  test('#elif __cplusplus enters a cxx-only region', () => {
+    const src = [
+      '#if SOMETHING',
+      '#include <a.h>',
+      '#elif __cplusplus',
+      '#include <b.h>',
+      '#endif',
+      '',
+    ].join('\n');
+    expect(scanHeader(src).includes).toEqual([
+      {token: 'a.h', cxxGuarded: false},
+      {token: 'b.h', cxxGuarded: true},
+    ]);
+  });
+});
+
+describe('scanHeader C++ / ObjC surface detection', () => {
+  test('an unguarded namespace is unguarded C++', () => {
+    const r = scanHeader('namespace facebook { struct X; }\n');
+    expect(r.hasUnguardedCxx).toBe(true);
+    expect(r.hasGuardedCxx).toBe(false);
+  });
+
+  test('a namespace under __cplusplus is guarded, not unguarded', () => {
+    const r = scanHeader('#ifdef __cplusplus\nnamespace facebook {}\n#endif\n');
+    expect(r.hasGuardedCxx).toBe(true);
+    expect(r.hasUnguardedCxx).toBe(false);
+  });
+
+  test('a named aggregate with a member initializer is ObjC++', () => {
+    const r = scanHeader('struct RCTFontProperties { CGFloat size = NAN; };\n');
+    expect(r.hasUnguardedCxx).toBe(true);
+  });
+
+  test('an ANONYMOUS aggregate with a member initializer is ObjC++', () => {
+    // Regression: the tag name is optional, so a typedef'd anonymous struct
+    // carrying a C++ default member initializer is still detected as ObjC++.
+    const r = scanHeader('typedef struct { CGFloat x = NAN; } Foo;\n');
+    expect(r.hasUnguardedCxx).toBe(true);
+  });
+
+  test('@interface marks the header as ObjC', () => {
+    const r = scanHeader('@interface RCTBridge : NSObject\n@end\n');
+    expect(r.hasObjC).toBe(true);
+  });
+});
+
+describe('scanHeader comment handling', () => {
+  test('a multi-line block comment mentioning C++ keywords does not trip the detector', () => {
+    const src = [
+      '/*',
+      ' * namespace foo is documented here',
+      ' * template <typename T> and constexpr too',
+      ' */',
+      '@interface RCTFoo',
+      '@end',
+      '',
+    ].join('\n');
+    const r = scanHeader(src);
+    expect(r.hasUnguardedCxx).toBe(false);
+    expect(r.hasGuardedCxx).toBe(false);
+    expect(r.hasObjC).toBe(true);
+  });
+
+  test('an inline block comment does not trip the detector', () => {
+    expect(scanHeader('int x; /* namespace y */\n').hasUnguardedCxx).toBe(
+      false,
+    );
+  });
+
+  test('a // line comment mentioning a keyword does not trip the detector', () => {
+    expect(scanHeader('// using namespace std;\n').hasUnguardedCxx).toBe(false);
+  });
+});

packages/react-native/scripts/ios-prebuild/headers-compose.js

@@ -26,10 +26,28 @@ const {
   renderReactModuleMap,
   renderUmbrellaHeader,
 } = require('./headers-spec');
-const {execSync} = require('child_process');
+const {execSync, execFileSync} = require('child_process');
+const crypto = require('crypto');
 const fs = require('fs');
 const path = require('path');
 
+// Hash of the compose tooling itself. Folded into the freshness marker so a
+// local edit to any of these scripts (which changes the composed output)
+// forces a recompose even when the cached source xcframework is untouched —
+// the Info.plist mtime alone can't catch that. Stable across fresh checkouts
+// (content-based, not mtime-based). sha256 to avoid weak-hash lint.
+function composeToolingHash() /*: string */ {
+  const hash = crypto.createHash('sha256');
+  for (const name of [
+    'headers-inventory.js',
+    'headers-spec.js',
+    'headers-compose.js',
+  ]) {
+    hash.update(fs.readFileSync(path.join(__dirname, name)));
+  }
+  return hash.digest('hex');
+}
+
 /*:: import type {HeadersSpecPlan, SpecEntry} from './headers-spec'; */
 
 /**
@@ -38,7 +56,20 @@ const path = require('path');
  * artifact must not be produced.
  */
 function computeSpecPlan(rnRoot /*: string */) /*: HeadersSpecPlan */ {
-  const plan = planFromInventory(computeInventory(rnRoot), rnRoot);
+  const inventory = computeInventory(rnRoot);
+  // R8, part 1: two distinct sources mapping to the same natural Headers/ path.
+  // addIdentity merges these and planFromInventory keeps only identities[0], so
+  // the second source would be silently dropped. Fail closed instead.
+  if (inventory.collisions.length > 0) {
+    const detail = inventory.collisions
+      .map(c => `${c.naturalPath} <- ${c.sources.join(', ')}`)
+      .join('\n  ');
+    throw new Error(
+      `header-inventory natural-path collisions (R8):\n  ${detail}`,
+    );
+  }
+  const plan = planFromInventory(inventory, rnRoot);
+  // R8, part 2: two natural paths colliding on the same xcframework destination.
   if (plan.collisions.length > 0) {
     throw new Error(
       `headers-spec collisions (R8):\n  ${plan.collisions.join('\n  ')}`,
@@ -106,7 +137,7 @@ function emitReactFrameworkHeaders(
   for (const slice of slices) {
     const fwk = path.join(xcfwPath, slice, 'React.framework');
     fs.rmSync(path.join(fwk, 'Headers'), {recursive: true, force: true});
-    execSync(`/bin/cp -Rc "${stage}" "${path.join(fwk, 'Headers')}"`);
+    execFileSync('/bin/cp', ['-Rc', stage, path.join(fwk, 'Headers')]);
     fs.rmSync(path.join(fwk, 'Modules'), {recursive: true, force: true});
     fs.mkdirSync(path.join(fwk, 'Modules'), {recursive: true});
     fs.writeFileSync(
@@ -187,7 +218,7 @@ function buildReactNativeHeadersXcframework(
           `incomplete ReactNativeHeaders.xcframework.`,
       );
     }
-    execSync(`/bin/cp -Rc "${src}" "${path.join(stage, ns)}"`);
+    execFileSync('/bin/cp', ['-Rc', src, path.join(stage, ns)]);
   }
   // Set equality with the deps artifact: a namespace dir present in the
   // artifact but neither declared for relocation (DEPS_NAMESPACES) nor
@@ -218,10 +249,15 @@ function buildReactNativeHeadersXcframework(
   if (hermesHeaders != null) {
     const src = path.join(hermesHeaders, 'hermes');
     if (fs.existsSync(src)) {
-      execSync(`/bin/cp -Rc "${src}" "${path.join(stage, 'hermes')}"`);
+      execFileSync('/bin/cp', ['-Rc', src, path.join(stage, 'hermes')]);
       hermesFolded = true;
     } else {
-      console.warn(`headers-compose: hermes headers missing at ${src}`);
+      console.warn(
+        `headers-compose: hermes headers missing at ${src} — the composed ` +
+          'ReactNativeHeaders will NOT resolve <hermes/...>; a downstream ' +
+          'build that imports Hermes headers will fail. Ensure the hermes-ios ' +
+          "tarball's destroot/include was staged into the slot.",
+      );
     }
   }
   // R10: per-namespace umbrella headers (e.g. React_RCTAppDelegate-umbrella.h)
@@ -331,8 +367,10 @@ function ensureHeadersLayout(
   const sourceStat = fs.statSync(path.join(sourceXcfw, 'Info.plist'));
   // Fold the hermes-headers presence into the marker so a slot that gains
   // staged hermes headers (e.g. after a tooling upgrade re-downloads them)
-  // recomposes instead of reusing a hermes-less ReactNativeHeaders.
-  const marker = `${sourceXcfw}\n${sourceStat.mtimeMs}\n${hermesHeaders ?? 'no-hermes'}\n`;
+  // recomposes instead of reusing a hermes-less ReactNativeHeaders. The
+  // compose-tooling hash makes a local edit to headers-{inventory,spec,compose}
+  // recompose too (the source xcframework's Info.plist mtime can't detect that).
+  const marker = `${sourceXcfw}\n${sourceStat.mtimeMs}\n${hermesHeaders ?? 'no-hermes'}\ntooling:${composeToolingHash()}\n`;
   if (
     !force &&
     fs.existsSync(reactXcfw) &&
@@ -349,7 +387,7 @@ function ensureHeadersLayout(
   fs.rmSync(reactXcfw, {recursive: true, force: true});
   fs.rmSync(markerPath, {force: true});
   fs.mkdirSync(outDir, {recursive: true});
-  execSync(`/bin/cp -Rc "${sourceXcfw}" "${reactXcfw}"`);
+  execFileSync('/bin/cp', ['-Rc', sourceXcfw, reactXcfw]);
   fs.rmSync(path.join(reactXcfw, '_CodeSignature'), {
     recursive: true,
     force: true,

packages/react-native/scripts/ios-prebuild/headers-inventory.js

@@ -158,8 +158,30 @@ function scanHeader(text /*: string */) /*: {
   const cxxRe =
     /^\s*(namespace\s+[A-Za-z_]|template\s*<|extern\s+"C\+\+"|enum\s+class\b|constexpr\b|using\s+(namespace\s|[A-Za-z_]\w*\s*=))/;
 
+  // Track /* ... */ block comments across lines so a documentation line inside
+  // a comment (e.g. `namespace`, `template <`, `constexpr`) can't trip the C++
+  // detector below and needlessly shrink the umbrella.
+  let inBlockComment = false;
   for (const rawLine of text.split('\n')) {
-    const line = rawLine.replace(/\/\/.*$/, '');
+    let line = rawLine;
+    if (inBlockComment) {
+      const end = line.indexOf('*/');
+      if (end === -1) {
+        continue; // whole line still inside a block comment
+      }
+      line = line.slice(end + 2);
+      inBlockComment = false;
+    }
+    // Drop complete inline block comments, then line comments (which also
+    // swallow any `/*` living inside a `//` comment), then detect a block
+    // comment that opens and runs onto the next line.
+    line = line.replace(/\/\*.*?\*\//g, '');
+    line = line.replace(/\/\/.*$/, '');
+    const blockOpen = line.indexOf('/*');
+    if (blockOpen !== -1) {
+      inBlockComment = true;
+      line = line.slice(0, blockOpen);
+    }
     const cond = line.match(/^\s*#\s*(if|ifdef|ifndef|elif|else|endif)\b(.*)$/);
     if (cond) {
       const [, directive, rest] = cond;
@@ -214,9 +236,11 @@ function scanHeader(text /*: string */) /*: {
   // declaration carrying an `=` initializer. Whole-text (not per-line) so the
   // aggregate context is required, avoiding false positives on file-scope
   // definitions. Unguarded by construction (definitions can't sit under a
-  // pure `#ifdef __cplusplus` and still be the ObjC surface).
+  // pure `#ifdef __cplusplus` and still be the ObjC surface). The tag name is
+  // optional so an anonymous `typedef struct { CGFloat x = NAN; } Foo;` is
+  // caught too (a named aggregate is not required for the ObjC++ surface).
   const aggregateMemberInitRe =
-    /\b(?:struct|class)\s+[A-Za-z_]\w*[^;{}]*\{[^{}]*?\b[A-Za-z_][\w\s:<>,]*\**\s+\*?[A-Za-z_]\w*\s*=\s*[^;{}]+;/s;
+    /\b(?:struct|class)\b(?:\s+[A-Za-z_]\w*)?[^;{}]*\{[^{}]*?\b[A-Za-z_][\w\s:<>,]*\**\s+\*?[A-Za-z_]\w*\s*=\s*[^;{}]+;/s;
   if (aggregateMemberInitRe.test(text)) {
     hasUnguardedCxx = true;
   }
@@ -325,7 +349,7 @@ function buildInventory(rootFolder /*: string */) /*: {
     const headerMaps = podSpecsWithHeaderFiles[podspecPath];
     // xcframework.js and vfs.js both use the ROOT spec's name (first map) as
     // the pod folder, with the same first-occurrence '-' -> '_' replacement.
-    const podName = headerMaps[0].specName.replace('-', '_');
+    const podName = headerMaps[0].specName.replace(/-/g, '_');
 
     for (const headerMap of headerMaps) {
       for (const header of headerMap.headers) {
@@ -578,15 +602,21 @@ if (require.main === module) {
  * without going through the JSON manifest on disk (e.g. the prebuild compose
  * step feeding headers-spec.planFromInventory).
  */
-function computeInventory(
-  rootFolder /*: string */,
-) /*: {headers: Array<HeaderEntry>} */ {
-  const {entries, sourceToNatural} = buildInventory(rootFolder);
+function computeInventory(rootFolder /*: string */) /*: {
+  headers: Array<HeaderEntry>,
+  collisions: Array<{naturalPath: string, sources: Array<string>}>,
+} */ {
+  const {entries, sourceToNatural, collisions} = buildInventory(rootFolder);
   classifyEntries(entries, sourceToNatural, rootFolder);
   return {
     headers: Array.from(entries.values()).sort((a, b) =>
       a.naturalPath.localeCompare(b.naturalPath),
     ),
+    // Natural-path collisions (two distinct sources mapping to the same
+    // Headers/ path) are silently merged by addIdentity — planFromInventory
+    // only keeps identities[0].source, so the second source is dropped. Surface
+    // them so the compose gate can fail closed (R8) instead of regressing.
+    collisions,
   };
 }
 

packages/react-native/scripts/ios-prebuild/headers-spec.js

@@ -56,6 +56,13 @@
  *     header manifest).
  * R8. Collisions are ERRORS: two different source files may never project to
  *     the same destination path.
+ * R9. Private React headers — a curated allowlist of `<React/...>` headers the
+ *     umbrella (R4) excludes (they are `+`-suffixed and/or objc-blocked) — are
+ *     added to the React framework module map so privileged consumers can reach
+ *     them. The allowlist fails closed on drift (validatePrivateReactHeaders).
+ * R10. Per-namespace umbrella headers are emitted into ReactNativeHeaders so
+ *     consumers that import a whole namespace (e.g. Expo) get one entry point;
+ *     each is derived from namespaceModules (R5) so it cannot drift.
  * R11. ONE source file, ONE content location. Some sources ship under several
  *     spellings (React/X.h + a legacy pod-namespace form like CoreModules/X.h,
  *     or a bare root alias + React_RCTAppDelegate/X.h). Under the VFS overlay

packages/react-native/scripts/ios-prebuild/xcframework.js

@@ -18,7 +18,7 @@ const childProcess = require('child_process');
 const fs = require('fs');
 const path = require('path');
 
-const {execSync} = childProcess;
+const {execSync, execFileSync} = childProcess;
 const {createLogger} = utils;
 
 const frameworkLog = createLogger('XCFramework');
@@ -125,11 +125,17 @@ function buildXCFrameworks(
     // reactnative-core artifact so the React-Core-prebuilt pod can vend both
     // (React.framework -> <React/...>, ReactNativeHeaders -> every other
     // namespace). The headers-only xcframework is a sibling of React.xcframework.
-    execSync(
-      `tar -czf ${tarFilePath} -C ${path.dirname(outputPath)} React.xcframework ${path.basename(headersXcfw)}`,
-      {
-        stdio: 'inherit',
-      },
+    execFileSync(
+      'tar',
+      [
+        '-czf',
+        tarFilePath,
+        '-C',
+        path.dirname(outputPath),
+        'React.xcframework',
+        path.basename(headersXcfw),
+      ],
+      {stdio: 'inherit'},
     );
   } catch (error) {
     frameworkLog(
@@ -148,8 +154,15 @@ function buildXCFrameworks(
   );
   frameworkLog('Creating tar file: ' + headersTarPath);
   try {
-    execSync(
-      `tar -czf ${headersTarPath} -C ${path.dirname(headersXcfw)} ReactNativeHeaders.xcframework`,
+    execFileSync(
+      'tar',
+      [
+        '-czf',
+        headersTarPath,
+        '-C',
+        path.dirname(headersXcfw),
+        'ReactNativeHeaders.xcframework',
+      ],
       {stdio: 'inherit'},
     );
   } catch (error) {