Merge branch 'main' of https://github.com/MetaMask/metamask-mobile into refactor/headerstandard-titles-views

brianacnguyen · brianacnguyen · commit 59741b0d2379 · 2026-05-05T17:08:05.000-07:00
diff --git a/bitrise.yml b/bitrise.yml
@@ -3597,9 +3597,9 @@ trigger_map:
   # Disable auto RC generation
   # - push_branch: release/*
   #   pipeline: pr_rc_rwy_pipeline
-  - push_branch: main
-    pipeline: expo_dev_pipeline
-  - tag: 'qa-*'
-    pipeline: create_qa_builds_pipeline
-  - tag: 'v*.*.*'
-    pipeline: create_qa_builds_pipeline
+  # - push_branch: main
+  #   pipeline: expo_dev_pipeline
+  # - tag: 'qa-*'
+  #   pipeline: create_qa_builds_pipeline
+  # - tag: 'v*.*.*'
+  #   pipeline: create_qa_builds_pipeline
diff --git a/scripts/lib/validate-value.js b/scripts/lib/validate-value.js
@@ -0,0 +1,187 @@
+/**
+ * Shared hygiene checks for CI-injected values (GitHub secrets and builds.yml env values).
+ *
+ * The goal is to catch operator mistakes (trailing newline, Windows line endings,
+ * invisible characters pasted from rich text editors, etc.) at build time, before
+ * malformed values end up baked into a production binary.
+ *
+ * Intentionally format-agnostic: it does not try to understand whether a value
+ * is a URL, base64 blob, JWT, etc. It only enforces generic hygiene.
+ *
+ * Usage:
+ *   const { checkValue } = require('./lib/validate-value');
+ *   const issues = checkValue('MM_SENTRY_DSN', value);
+ *   // issues is an array; empty => value is clean.
+ *
+ * Output contract: issue messages MUST NOT include the value itself or any
+ * substring of it. Only the length, offsets, and character code points are
+ * safe to surface.
+ */
+
+/* global Buffer */
+
+// eslint-disable-next-line no-misleading-character-class -- intentional set of invisible code points (ZWSP/ZWNJ/ZWJ/BOM)
+const ZERO_WIDTH_CHARS = /[\u200B\u200C\u200D\uFEFF]/;
+
+// C0/C1 control chars, excluding tab (\u0009), line feed (\u000A), and
+// carriage return (\u000D). CR is reported separately with a friendlier
+// message. LF is allowed mid-value for multi-line secrets (PEM keys, base64).
+/* eslint-disable no-control-regex -- intentionally matches control characters to flag them */
+const CONTROL_CHARS =
+  /[\u0001-\u0008\u000B\u000C\u000E-\u001F\u007F-\u009F]/;
+/* eslint-enable no-control-regex */
+
+function formatCodePoint(ch) {
+  return `U+${ch.charCodeAt(0).toString(16).padStart(4, '0').toUpperCase()}`;
+}
+
+/**
+ * @param {string} name - Identifier to report in violations (e.g. secret name or env key).
+ * @param {unknown} value - The value to check. Non-string values are coerced via String().
+ * @param {object} [options]
+ * @param {boolean} [options.allowEmpty=false] - If true, an empty string is not a violation.
+ *   Whitespace-only strings still fail even when allowEmpty is true (they indicate a typo).
+ * @returns {{ code: string, message: string }[]} - One entry per distinct violation; empty array means clean.
+ */
+function checkValue(name, value, options = {}) {
+  const { allowEmpty = false } = options;
+  const violations = [];
+
+  /**
+   * `missing`: the value is `undefined` or `null`. For secrets, this means the
+   * referenced GitHub Environment secret was never set (or the Environment
+   * itself is misconfigured). For YAML env entries, it usually means a key
+   * like `FOO:` was written with no value, which js-yaml parses as `null`.
+   * Short-circuits: nothing else can be checked without a value.
+   */
+  if (value === undefined || value === null) {
+    violations.push({
+      code: 'missing',
+      message: `${name}: value is null or not defined`,
+    });
+    return violations;
+  }
+
+  const str = String(value);
+  const len = Buffer.byteLength(str, 'utf8');
+
+  /**
+   * `empty`: the value is the empty string `""`. Skipped when the caller
+   * passes `{ allowEmpty: true }`, used for intentionally-empty YAML entries
+   * such as optional allowlists (e.g. `MM_PERPS_HIP3_ALLOWLIST_MARKETS: ''`).
+   * Short-circuits: the remaining checks don't apply to an empty string.
+   */
+  if (str === '') {
+    if (!allowEmpty) {
+      violations.push({
+        code: 'empty',
+        message: `${name}: value is an empty string`,
+      });
+    }
+    return violations;
+  }
+
+  /**
+   * `whitespace_only`: the value is non-empty but contains nothing except
+   * whitespace. Almost always a typo (e.g. someone pasted a single space
+   * into the GitHub Secret UI). Fails even with `allowEmpty: true`, because
+   * "intentionally empty" should be `""`, not `"   "`.
+   * Short-circuits: the value has no meaningful content to inspect further.
+   */
+  if (str.trim() === '') {
+    violations.push({
+      code: 'whitespace_only',
+      message: `${name}: value is whitespace-only (${len} bytes)`,
+    });
+    return violations;
+  }
+
+  /**
+   * `leading_whitespace`: the value begins with whitespace (space, tab, LF,
+   * etc.). Accidental leading whitespace breaks URL parsing, base64 decoding,
+   * and token comparisons — almost never intentional.
+   */
+  if (/^\s/.test(str)) {
+    violations.push({
+      code: 'leading_whitespace',
+      message: `${name}: value has leading whitespace (${len} bytes total)`,
+    });
+  }
+
+  /**
+   * `trailing_whitespace`: the value ends with whitespace. This is the
+   * single most common real-world paste mistake — e.g. copying a token from
+   * a terminal or editor ends up including the trailing `\n`. It's the
+   * specific failure mode this entire module was built to catch.
+   */
+  if (/\s$/.test(str)) {
+    violations.push({
+      code: 'trailing_whitespace',
+      message: `${name}: value has trailing whitespace (${len} bytes total); a trailing newline pasted from a terminal or editor is the most common cause`,
+    });
+  }
+
+  /**
+   * `carriage_return`: the value contains `\r` anywhere. CR is not part of
+   * the base64 alphabet, PEM uses LF, and no sanctioned secret format needs
+   * it — its presence is essentially always an artifact of Windows CRLF line
+   * endings surviving a copy-paste. Reported separately from `control_chars`
+   * so the remediation message can specifically call out Windows endings.
+   */
+  const crIndex = str.indexOf('\r');
+  if (crIndex !== -1) {
+    violations.push({
+      code: 'carriage_return',
+      message: `${name}: value contains a carriage return (\\r) at offset ${crIndex}/${len}; strip Windows line endings before saving the secret`,
+    });
+  }
+
+  /**
+   * `nul_byte`: the value contains `\u0000`. Never legitimate in any secret
+   * format we use. A NUL byte can terminate strings prematurely in C-based
+   * tooling and is a classic source of silent truncation bugs.
+   */
+  const nulIndex = str.indexOf('\u0000');
+  if (nulIndex !== -1) {
+    violations.push({
+      code: 'nul_byte',
+      message: `${name}: value contains a NUL byte at offset ${nulIndex}/${len}`,
+    });
+  }
+
+  /**
+   * `control_chars`: the value contains any other C0/C1 control character
+   * (range defined in CONTROL_CHARS — excludes tab, LF, and CR, which are
+   * allowed or reported separately). Hits here usually indicate non-text
+   * binary data was pasted as if it were a string, or a stray escape
+   * sequence. The message includes the specific code point (e.g. U+0007)
+   * so operators can identify what was pasted.
+   */
+  const ctrlMatch = CONTROL_CHARS.exec(str);
+  if (ctrlMatch) {
+    violations.push({
+      code: 'control_chars',
+      message: `${name}: value contains control character ${formatCodePoint(ctrlMatch[0])} at offset ${ctrlMatch.index}/${len}`,
+    });
+  }
+
+  /**
+   * `zero_width`: the value contains an invisible Unicode character —
+   * zero-width space (U+200B), zero-width non-joiner (U+200C), zero-width
+   * joiner (U+200D), or byte-order mark (U+FEFF). These are introduced when
+   * copying from rich-text sources (Google Docs, Slack, Notion, Confluence)
+   * and are invisible to the human eye but break exact-match comparisons
+   * and break formats like base64 that only accept a strict alphabet.
+   */
+  const zwMatch = ZERO_WIDTH_CHARS.exec(str);
+  if (zwMatch) {
+    violations.push({
+      code: 'zero_width',
+      message: `${name}: value contains invisible character ${formatCodePoint(zwMatch[0])} at offset ${zwMatch.index}/${len}; likely pasted from a rich text source`,
+    });
+  }
+
+  return violations;
+}
+
+module.exports = { checkValue };
diff --git a/scripts/lib/validate-value.test.js b/scripts/lib/validate-value.test.js
@@ -0,0 +1,145 @@
+const { checkValue } = require('./validate-value');
+
+const codes = (issues) => issues.map((i) => i.code);
+
+describe('checkValue', () => {
+  describe('happy path', () => {
+    it('returns no issues for a typical single-line secret', () => {
+      expect(checkValue('X', 'key_live_abc123xyz')).toEqual([]);
+    });
+
+    it('returns no issues for a multi-line base64 blob (embedded \\n mid-value is allowed)', () => {
+      const b64 = 'eyJhcGlLZXkiOiJhYmMifQ==\nsecond-line-content==';
+      expect(checkValue('GOOGLE_SERVICES_B64_IOS', b64)).toEqual([]);
+    });
+
+    it('coerces non-string values via String()', () => {
+      expect(checkValue('N', 42)).toEqual([]);
+      expect(checkValue('B', true)).toEqual([]);
+    });
+  });
+
+  describe('missing / empty', () => {
+    it('flags undefined', () => {
+      expect(codes(checkValue('X', undefined))).toEqual(['missing']);
+    });
+
+    it('flags null', () => {
+      expect(codes(checkValue('X', null))).toEqual(['missing']);
+    });
+
+    it('flags empty string by default', () => {
+      expect(codes(checkValue('X', ''))).toEqual(['empty']);
+    });
+
+    it('allows empty string when allowEmpty=true', () => {
+      expect(checkValue('X', '', { allowEmpty: true })).toEqual([]);
+    });
+
+    it('flags whitespace-only even when allowEmpty=true', () => {
+      expect(codes(checkValue('X', '   ', { allowEmpty: true }))).toEqual([
+        'whitespace_only',
+      ]);
+    });
+  });
+
+  describe('leading / trailing whitespace', () => {
+    it('flags trailing newline (the classic paste mistake)', () => {
+      expect(codes(checkValue('X', 'value\n'))).toEqual([
+        'trailing_whitespace',
+      ]);
+    });
+
+    it('flags trailing space', () => {
+      expect(codes(checkValue('X', 'value '))).toEqual([
+        'trailing_whitespace',
+      ]);
+    });
+
+    it('flags trailing tab', () => {
+      expect(codes(checkValue('X', 'value\t'))).toEqual([
+        'trailing_whitespace',
+      ]);
+    });
+
+    it('flags leading space', () => {
+      expect(codes(checkValue('X', ' value'))).toEqual([
+        'leading_whitespace',
+      ]);
+    });
+
+    it('flags both leading and trailing whitespace in one pass', () => {
+      expect(codes(checkValue('X', ' value '))).toEqual([
+        'leading_whitespace',
+        'trailing_whitespace',
+      ]);
+    });
+  });
+
+  describe('control characters', () => {
+    it('flags any \\r (Windows line endings)', () => {
+      expect(codes(checkValue('X', 'abc\r\ndef'))).toContain('carriage_return');
+    });
+
+    it('flags a standalone \\r mid-value', () => {
+      expect(codes(checkValue('X', 'abc\rdef'))).toContain('carriage_return');
+    });
+
+    it('flags NUL bytes', () => {
+      expect(codes(checkValue('X', 'abc\u0000def'))).toContain('nul_byte');
+    });
+
+    it('flags other C0 control characters', () => {
+      expect(codes(checkValue('X', 'abc\u0007def'))).toContain(
+        'control_chars',
+      );
+    });
+
+    it('flags DEL (U+007F)', () => {
+      expect(codes(checkValue('X', 'abc\u007Fdef'))).toContain(
+        'control_chars',
+      );
+    });
+
+    it('does NOT flag tab mid-value', () => {
+      expect(checkValue('X', 'abc\tdef')).toEqual([]);
+    });
+
+    it('does NOT flag LF mid-value (allowed for PEM / base64)', () => {
+      expect(checkValue('X', 'abc\ndef')).toEqual([]);
+    });
+  });
+
+  describe('invisible characters', () => {
+    it('flags zero-width space', () => {
+      expect(codes(checkValue('X', 'abc\u200Bdef'))).toContain('zero_width');
+    });
+
+    it('flags BOM', () => {
+      expect(codes(checkValue('X', '\uFEFFvalue'))).toEqual(
+        expect.arrayContaining(['leading_whitespace', 'zero_width']),
+      );
+    });
+
+    it('flags zero-width joiner', () => {
+      expect(codes(checkValue('X', 'abc\u200Ddef'))).toContain('zero_width');
+    });
+  });
+
+  describe('message safety', () => {
+    it('never includes the value in the message', () => {
+      const secret = 'super-secret-token-xyz';
+      const issues = checkValue('X', `${secret}\n`);
+      for (const { message } of issues) {
+        expect(message).not.toContain(secret);
+      }
+    });
+
+    it('includes the name, a byte length, and the code in the output', () => {
+      const [issue] = checkValue('MM_SENTRY_DSN', 'abc ');
+      expect(issue.code).toBe('trailing_whitespace');
+      expect(issue.message).toContain('MM_SENTRY_DSN');
+      expect(issue.message).toMatch(/\d+ bytes/);
+    });
+  });
+});
diff --git a/scripts/validate-build-config.js b/scripts/validate-build-config.js
@@ -6,9 +6,21 @@
 const fs = require('fs');
 const path = require('path');
 const yaml = require('js-yaml');
+const { checkValue } = require('./lib/validate-value');
 
 const BUILDS_PATH = path.join(__dirname, '../builds.yml');
 
+// Env keys in builds.yml that may legitimately be the empty string. Every
+// other declared env key must have a non-empty value, so an accidental
+// `PORTFOLIO_API_URL: ''` (or a botched YAML anchor merge) fails validation
+// instead of shipping with a blank value. Adding to this list should be
+// deliberate: the review on that PR is the gate for "yes, this key is
+// intentionally optional."
+const ENV_KEYS_ALLOWED_EMPTY = new Set([
+  'MM_PERPS_HIP3_ALLOWLIST_MARKETS',
+  'MM_PERPS_HIP3_BLOCKLIST_MARKETS',
+]);
+
 function validate() {
   if (!fs.existsSync(BUILDS_PATH)) {
     console.error('❌ builds.yml not found');
@@ -46,6 +58,20 @@ function validate() {
     if (!build.github_environment) {
       errors.push(`${name}: missing github_environment`);
     }
+
+    // Hygiene checks on env values: catch trailing whitespace, stray \r,
+    // invisible characters, accidental empties, etc. so operator typos fail
+    // CI before the build fans out. Strict by default; only the keys in
+    // ENV_KEYS_ALLOWED_EMPTY may be the empty string. Whitespace-only values
+    // always fail, since "intentionally empty" should be `''` not `'   '`.
+    if (build.env && typeof build.env === 'object') {
+      for (const [envKey, envVal] of Object.entries(build.env)) {
+        const issues = checkValue(`${name}.env.${envKey}`, envVal, {
+          allowEmpty: ENV_KEYS_ALLOWED_EMPTY.has(envKey),
+        });
+        issues.forEach((issue) => errors.push(issue.message));
+      }
+    }
   });
 
   if (errors.length > 0) {
diff --git a/scripts/validate-secrets-from-config.js b/scripts/validate-secrets-from-config.js
