Hand the App Store Connect key to the clients through the environment

.github/workflows/ios-ship.yml

@@ -85,6 +85,7 @@ on:
       - 'scripts/ios-appstore-gm-prepare.sh'
       - 'scripts/ios-install-appstore-profiles.sh'
       - 'scripts/ios-stage-asc-key.sh'
+      - 'scripts/ios-load-asc-key.sh'
   workflow_dispatch:
   schedule:
     # Twice an hour, offset from other fleet apps
@@ -270,17 +271,29 @@ jobs:
           fi
           bash "$GITHUB_WORKSPACE/scripts/ios-appstore-gm-prepare.sh"
 
+      # The clients take the credential from the environment and never from a
+      # path (Kody rule 42), so the staged key is handed to each of them
+      # through its environment.  The loader reads the file the load step
+      # staged and exports ASC_KEY_P8 into this step's shell; the value never
+      # enters $GITHUB_ENV, never becomes an argument and never reaches a log
+      # line.
       - name: Enable CodeCaps APNs and verify production push profile
-        run: node "$GITHUB_WORKSPACE/scripts/ios-enable-apns.mjs"
+        run: |
+          source scripts/ios-load-asc-key.sh
+          node "$GITHUB_WORKSPACE/scripts/ios-enable-apns.mjs"
 
       - name: Install App Store profiles
-        run: bash "$GITHUB_WORKSPACE/scripts/ios-install-appstore-profiles.sh"
+        run: |
+          source scripts/ios-load-asc-key.sh
+          bash "$GITHUB_WORKSPACE/scripts/ios-install-appstore-profiles.sh"
 
       - name: Ship CodeCaps to TestFlight
         env:
           IOS_TF_RELEASE_NOTES: "1"
           IOS_MANUAL_SIGN: "1"
-        run: bash scripts/ios-ship-testflight.sh
+        run: |
+          source scripts/ios-load-asc-key.sh
+          bash scripts/ios-ship-testflight.sh
 
       - name: Sync standing TestFlight testers
         if: success()
@@ -302,17 +315,12 @@ jobs:
           set +e
           # The client resolves the signing key from its environment and never
           # from a path (Kody rule 42), so the staged key is handed to the
-          # child through the child's environment by the hardened reader.  The
-          # key stays out of argv, out of stdout, out of $GITHUB_ENV and out
-          # of the log.
-          if [ -n "${ASC_KEY_PATH:-}" ]; then
-            node scripts/ios-fleet/asc-key.mjs exec "$ASC_KEY_PATH" -- \
-              node scripts/ios-fleet/asc-api.mjs ensure-standing-testers "$appleid"
-            rc=$?
-          else
-            node scripts/ios-fleet/asc-api.mjs ensure-standing-testers "$appleid"
-            rc=$?
-          fi
+          # child through the child's environment by the loader.  The key
+          # stays out of argv, out of stdout, out of $GITHUB_ENV and out of
+          # the log.
+          source scripts/ios-load-asc-key.sh
+          node scripts/ios-fleet/asc-api.mjs ensure-standing-testers "$appleid"
+          rc=$?
           set -e
           if [ "$rc" -ne 0 ]; then
             echo "::warning title=TestFlight standing testers::tester sync exited ${rc}; see the standing-testers lines above.  The TestFlight upload itself succeeded."

scripts/ios-load-asc-key.sh

@@ -0,0 +1,82 @@
+#!/usr/bin/env bash
+# Load the staged App Store Connect private key into ASC_KEY_P8.
+#
+# Source this file from a workflow step that starts an App Store Connect
+# client:
+#
+#   source scripts/ios-load-asc-key.sh
+#   node scripts/ios-fleet/asc-api.mjs ensure-standing-testers "$appleid"
+#
+# The workflow stages the key once, to a mode-600 file, and this helper reads
+# that file back into the calling shell as ASC_KEY_P8.  The credential then
+# travels to the client as the environment variable the client reads, so it
+# never enters $GITHUB_ENV, never becomes a command argument and never
+# reaches a log line.  It exists only in the environment of the step that
+# sourced it and in the environment of the clients that step starts.
+#
+# The clients refuse a path as a credential source (Kody rule 42), so the
+# staged file stays what it is:  a staging detail for the Apple tools that
+# need a key on disk, and the carrier between two steps of the same job.
+# Nothing here is a second way for a client to resolve a credential; it is
+# the shell layer that owns the credential handing it over.
+#
+# This file is meant to be sourced, not executed.  It sets no shell options,
+# because a sourced file that did would change the caller's shell.
+
+# Reads the staged key and exports it as ASC_KEY_P8.
+#
+# The checks are the ones a credential deserves on the way in:  the path must
+# be set, the file must exist, its mode must be 600 so that nothing else on
+# the runner can read it, and it must hold something.  Each failure names
+# what is wrong and returns non-zero, and none of them prints the value.
+asc_load_asc_key() {
+  local path="${ASC_KEY_PATH:-}"
+  if [ -z "$path" ]; then
+    echo "error: ASC_KEY_PATH is not set; the ASC key was never staged." >&2
+    return 1
+  fi
+  if [ ! -f "$path" ]; then
+    echo "error: the staged ASC key file is missing." >&2
+    return 1
+  fi
+
+  # The file holds a credential, so only its owner may read it.  The mode is
+  # the last chance to notice, so a file that is group or world readable is
+  # refused rather than loaded.
+  #
+  # BSD and GNU stat disagree on the flag, and they are told apart by the
+  # platform rather than tried in turn:  GNU stat's -f does not take a
+  # permissions format, so it prints a filesystem report to stdout and then
+  # fails, and that report would be captured as the mode and would refuse a
+  # perfectly good file.
+  local mode=""
+  case "$(uname -s)" in
+    Darwin) mode="$(stat -f '%Lp' "$path" 2>/dev/null || true)" ;;
+    *) mode="$(stat -c '%a' "$path" 2>/dev/null || true)" ;;
+  esac
+  case "$mode" in
+    600) ;;
+    '')
+      echo "error: could not read the mode of the staged ASC key file; refusing to load it." >&2
+      return 1
+      ;;
+    *)
+      echo "error: the staged ASC key file is mode ${mode}, not 600; refusing to load it." >&2
+      return 1
+      ;;
+  esac
+
+  local pem
+  pem="$(cat "$path")"
+  if [ -z "${pem//[[:space:]]/}" ]; then
+    echo "error: the staged ASC key file holds no key material." >&2
+    return 1
+  fi
+
+  ASC_KEY_P8="$pem"
+  export ASC_KEY_P8
+  unset pem
+  return 0
+}
+
+asc_load_asc_key

scripts/ios-ship-workflow.test.mjs

@@ -114,7 +114,7 @@ test("ios-ship.yml ships automatically on a merge that touches the native tree",
   // What to Test is mandatory and published, never dry-rendered.
   assert.match(code, /IOS_TF_RELEASE_NOTES:\s*"1"/);
   // Ship through the wrapper that runs ship-testflight.sh (ensure-tf-ready inside).
-  assert.match(code, /run: bash scripts\/ios-ship-testflight\.sh\s*$/m);
+  assert.match(code, /bash scripts\/ios-ship-testflight\.sh\s*$/m);
   assert.match(read("scripts/ios-fleet/ship-testflight.sh"), /ensure-tf-ready/);
   // Hosted macOS with the stable-Xcode guard, never a self-hosted label.
   assert.match(code, /runs-on:\s*macos-latest/);
@@ -251,6 +251,47 @@ test("ASC credentials come from Fleet-OPS /asc, never the stale repository secre
   assert.doesNotMatch(prepare, /echo "\$IOS_DIST_P12/);
 });
 
+test("the ASC credential reaches every client through the environment, never a path", () => {
+  // The clients refuse a path as a credential source (Kody rule 42), so the
+  // shell layer that owns the credential has to hand it over in the
+  // environment.  Every step that starts a client sources the loader first,
+  // or the step runs with no credential at all and the failure reads like a
+  // permission problem.
+  const clients = [
+    "ios-enable-apns.mjs",
+    "ios-install-appstore-profiles.sh",
+    "ios-ship-testflight.sh",
+    "asc-api.mjs",
+  ];
+  const blocks = [...code.matchAll(/^[ ]*run: \|\n((?:[ ]{10,}.*\n|\n)*)/gm)].map((m) => m[1]);
+  assert.ok(blocks.length >= 6, `the workflow should have several run blocks, found ${blocks.length}`);
+  for (const client of clients) {
+    const starters = blocks.filter((b) => b.includes(client));
+    assert.ok(starters.length >= 1, `no run block starts ${client}`);
+    for (const block of starters) {
+      assert.match(
+        block,
+        /source scripts\/ios-load-asc-key\.sh/,
+        `a step that starts ${client} must hand it the credential through the environment`,
+      );
+    }
+  }
+
+  // The removed subcommand must not come back:  asc-key.mjs has no exec.
+  assert.doesNotMatch(code, /asc-key\.mjs exec/);
+  assert.doesNotMatch(read("scripts/ios-fleet/asc-key.mjs"), /"exec"/);
+
+  // The loader exports the key and never prints it.
+  const loader = read("scripts/ios-load-asc-key.sh");
+  assert.match(loader, /export ASC_KEY_P8/);
+  assert.doesNotMatch(loader, /echo "\$pem"/);
+  assert.doesNotMatch(loader, /echo "\$ASC_KEY_P8"/);
+  assert.doesNotMatch(loader, /printf[^\n]*\$pem/);
+
+  // The loader is part of the ship, so a change to it runs the ship.
+  assert.ok(pushPaths().includes("scripts/ios-load-asc-key.sh"), "editing the loader must re-ship");
+});
+
 test("the ship carries no standing gate override and always publishes What to Test", () => {
   assert.doesNotMatch(code, /IOS_TF_MIN_INTERVAL_SEC/);
   assert.doesNotMatch(code, /--force-ship/);
@@ -275,8 +316,20 @@ test("the marketing train is 1.0.N and release notes use the owner clock", () =>
 });
 
 test("scheduled gate and key handoff shell tests pass", () => {
-  for (const script of ["scripts/ios-fleet/test-scheduled-ship-gate.sh", "scripts/test-ios-stage-asc-key.sh"]) {
-    const out = execFileSync("bash", [join(ROOT, script)], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] });
+  for (const script of [
+    "scripts/ios-fleet/test-scheduled-ship-gate.sh",
+    "scripts/test-ios-stage-asc-key.sh",
+    "scripts/test-ios-load-asc-key.sh",
+  ]) {
+    // A shell test that fails says why on stderr, and the reason belongs in
+    // the failure message:  a CI log that only says "Command failed" is a
+    // dead end.
+    let out = "";
+    try {
+      out = execFileSync("bash", [join(ROOT, script)], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] });
+    } catch (error) {
+      assert.fail(`${script} failed: ${String(error.stderr || "").trim() || error.message}`);
+    }
     assert.match(out, /all tests passed/, `${script} did not report success`);
   }
 });

scripts/test-ios-load-asc-key.sh

@@ -0,0 +1,126 @@
+#!/usr/bin/env bash
+# Offline test for the ASC key handoff in .github/workflows/ios-ship.yml:
+# scripts/ios-load-asc-key.sh reads the staged key back into the step's
+# environment so the App Store Connect clients receive it as ASC_KEY_P8.
+# No Apple credential is used; every key below is a synthetic fixture.
+set -euo pipefail
+
+repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+tmp_dir="$(mktemp -d)"
+trap 'rm -rf "$tmp_dir"' EXIT
+mkdir -p "$tmp_dir/staging"
+export ASC_STAGING_ROOT="$tmp_dir/staging"
+export GITHUB_ENV="$tmp_dir/github-env"
+: > "$GITHUB_ENV"
+
+cat > "$tmp_dir/fixture" <<'KEY'
+SYNTHETIC-HEADER
+SYNTHETIC-MULTILINE-FIXTURE-DO-NOT-USE
+SYNTHETIC-FOOTER
+KEY
+
+# The loader refuses a path it cannot vouch for, and it says why without ever
+# printing the value.  Each case is a separate refusal.
+refuse() {
+  local what="$1" needle="$2"
+  if ( export ASC_KEY_PATH="$3"; source "$repo_root/scripts/ios-load-asc-key.sh" ) \
+      > "$tmp_dir/out" 2> "$tmp_dir/err"; then
+    echo "the loader accepted ${what}" >&2
+    exit 1
+  fi
+  if ! grep -Fq "$needle" "$tmp_dir/err"; then
+    echo "the loader refused ${what} without naming the reason" >&2
+    cat "$tmp_dir/err" >&2
+    exit 1
+  fi
+  if grep -Fq 'SYNTHETIC-MULTILINE-FIXTURE' "$tmp_dir/out" "$tmp_dir/err"; then
+    echo 'key material reached the output of a refused load' >&2
+    exit 1
+  fi
+}
+
+# Stage the key the way the workflow does, so the loader is exercised against
+# the file the real handoff produces.
+bash "$repo_root/scripts/ios-stage-asc-key.sh" < "$tmp_dir/fixture" > "$tmp_dir/stdout" 2> "$tmp_dir/stderr"
+key_path="$(sed -n 's/^ASC_KEY_PATH=//p' "$GITHUB_ENV")"
+[[ -f "$key_path" ]]
+expected="$(cat "$key_path")"
+printf '%s\n' "$expected" > "$tmp_dir/expected"
+
+# The happy path: the loader puts the staged key into the caller's
+# environment, where the clients look for it.
+(
+  export ASC_KEY_PATH="$key_path"
+  source "$repo_root/scripts/ios-load-asc-key.sh" > "$tmp_dir/load-stdout" 2> "$tmp_dir/load-stderr"
+  [[ "$ASC_KEY_P8" == "$expected" ]]
+)
+[[ ! -s "$tmp_dir/load-stdout" ]]
+[[ ! -s "$tmp_dir/load-stderr" ]]
+if grep -Fq 'SYNTHETIC-MULTILINE-FIXTURE' "$tmp_dir/load-stdout" "$tmp_dir/load-stderr"; then
+  echo 'key material reached the output of a successful load' >&2
+  exit 1
+fi
+
+# The value must reach a child process as an environment variable, because
+# that is the only way a client accepts a credential.
+(
+  export ASC_KEY_PATH="$key_path"
+  source "$repo_root/scripts/ios-load-asc-key.sh" > /dev/null 2>&1
+  node -e 'process.stdout.write((process.env.ASC_KEY_P8 || "") + "\n")' | cmp - "$tmp_dir/expected"
+)
+
+# The loader reads the file it was pointed at, so a different file yields a
+# different value.  Without this a loader that returned a constant would pass
+# every other case.
+cp "$key_path" "$tmp_dir/other.p8"
+printf 'SYNTHETIC-OTHER\n' > "$tmp_dir/other.p8"
+chmod 600 "$tmp_dir/other.p8"
+(
+  export ASC_KEY_PATH="$tmp_dir/other.p8"
+  source "$repo_root/scripts/ios-load-asc-key.sh" > /dev/null 2>&1
+  [[ "$ASC_KEY_P8" == "SYNTHETIC-OTHER" ]]
+)
+
+# An unset path, a missing file, a file anyone can read and an empty file are
+# all refused.
+refuse 'an unset path' 'ASC_KEY_PATH is not set' ''
+refuse 'a missing file' 'is missing' "$tmp_dir/no-such-file.p8"
+cp "$key_path" "$tmp_dir/loose.p8"
+chmod 644 "$tmp_dir/loose.p8"
+refuse 'a world-readable key file' 'not 600' "$tmp_dir/loose.p8"
+: > "$tmp_dir/empty.p8"
+chmod 600 "$tmp_dir/empty.p8"
+refuse 'an empty key file' 'no key material' "$tmp_dir/empty.p8"
+
+# The mode probe must be platform aware.  GNU stat's -f prints a filesystem
+# report to stdout when the format is not one it knows, and that report would
+# be captured as the mode, so a perfectly good file would be refused on
+# every Linux runner.
+if grep -E "stat -f" "$repo_root/scripts/ios-load-asc-key.sh" | grep -qv Darwin; then
+  echo 'the loader uses the BSD stat outside a Darwin branch' >&2
+  exit 1
+fi
+
+# The loader must not put the key into the job's environment file:  that file
+# feeds every later step, and the key belongs to one step at a time.  The
+# check is on the write, not on the word, because the loader explains in a
+# comment why it does not write there.
+if grep -Eq '>> *"?\$\{?GITHUB_ENV' "$repo_root/scripts/ios-load-asc-key.sh"; then
+  echo 'the loader writes to the job environment file' >&2
+  exit 1
+fi
+
+# The workflow hands the credential to every client through the environment,
+# and it never calls the removed asc-key.mjs exec subcommand.
+workflow="$repo_root/.github/workflows/ios-ship.yml"
+if grep -Eq 'asc-key\.mjs exec' "$workflow"; then
+  echo 'the workflow calls the removed asc-key.mjs exec subcommand' >&2
+  exit 1
+fi
+load_count="$(grep -c 'source scripts/ios-load-asc-key\.sh' "$workflow" || true)"
+if [[ "$load_count" -lt 4 ]]; then
+  echo "the workflow sources the loader in only ${load_count} steps; every client step needs it" >&2
+  exit 1
+fi
+
+echo 'ios-load-asc-key: all tests passed'