diff --git a/.github/workflows/ios-ship.yml b/.github/workflows/ios-ship.yml index 3488d76b..59ea8f4d 100644 --- a/.github/workflows/ios-ship.yml +++ b/.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." diff --git a/scripts/ios-load-asc-key.sh b/scripts/ios-load-asc-key.sh new file mode 100644 index 00000000..9b228079 --- /dev/null +++ b/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 diff --git a/scripts/ios-ship-workflow.test.mjs b/scripts/ios-ship-workflow.test.mjs index 0cbc694e..49723ce2 100644 --- a/scripts/ios-ship-workflow.test.mjs +++ b/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`); } }); diff --git a/scripts/test-ios-load-asc-key.sh b/scripts/test-ios-load-asc-key.sh new file mode 100755 index 00000000..49d0a0dc --- /dev/null +++ b/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'