diff --git a/.github/workflows/shared/apm.md b/.github/workflows/shared/apm.md index 4b5438153..47194cd1d 100644 --- a/.github/workflows/shared/apm.md +++ b/.github/workflows/shared/apm.md @@ -166,15 +166,21 @@ jobs: outputs: matrix: ${{ steps.compute.outputs.matrix }} steps: - # SECURITY (S3): never echo $groups, $matrix, or any matrix.group.* value - # in any apm-prep step. private-key is a real secret string here. + # SECURITY (S3): the matrix written to $GITHUB_OUTPUT below carries + # NO secret values -- only routing metadata (id, kind, index, owner, + # repositories, packages, has-app). Credentials are resolved per-row + # in the apm job via $GITHUB_ENV (see "Resolve credentials" step). + # This avoids GitHub Actions' cross-job output redaction filter + # (HostContext.SecretMasker.MaskSecrets in actions/runner), which + # silently strips any job output whose value contains a registered + # secret substring and emits "Skip output '' since it may + # contain secret." See: docs/.../security-guides/security-hardening. - name: Compute APM credential-group matrix id: compute env: AW_APM_PACKAGES: ${{ github.aw.import-inputs.packages }} AW_APM_APPS: ${{ github.aw.import-inputs.apps }} AW_APM_LEGACY_APP_ID: ${{ github.aw.import-inputs.app-id }} - AW_APM_LEGACY_PRIVATE_KEY: ${{ github.aw.import-inputs.private-key }} AW_APM_LEGACY_OWNER: ${{ github.aw.import-inputs.owner }} AW_APM_LEGACY_REPOS: ${{ github.aw.import-inputs.repositories }} run: | @@ -207,7 +213,6 @@ jobs: --argjson packages "$packages_json" \ --argjson apps "$apps_json" \ --arg legacy_id "$legacy_id" \ - --arg legacy_pk "${AW_APM_LEGACY_PRIVATE_KEY:-}" \ --arg legacy_owner "${AW_APM_LEGACY_OWNER:-}" \ --arg legacy_repos "${AW_APM_LEGACY_REPOS:-}" \ 'def slug(s): s | gsub("[^a-zA-Z0-9-]"; "-") | ascii_downcase | .[0:32]; @@ -215,12 +220,21 @@ jobs: g + (if (g.id // "") == "" then {id: ("auto-" + slug(g.owner // "default"))} else {} end); [ (if (($packages // []) | length) > 0 and $legacy_id == "" - then [{id:"default",("app-id"):"",("private-key"):"",owner:"",repositories:"",packages:$packages}] + then [{id:"default",kind:"default",index:0,owner:"",repositories:"",packages:$packages,("has-app"):"false"}] else [] end), (if $legacy_id != "" - then [with_id({id:"legacy",("app-id"):$legacy_id,("private-key"):$legacy_pk,owner:$legacy_owner,repositories:$legacy_repos,packages:($packages // [])})] + then [with_id({id:"legacy",kind:"legacy",index:0,owner:$legacy_owner,repositories:$legacy_repos,packages:($packages // []),("has-app"):"true"})] else [] end), - (($apps // []) | map(with_id(.))) + (($apps // []) | to_entries | map( + with_id({ + id: (.value.id // ""), + kind: "apps", + index: .key, + owner: (.value.owner // ""), + repositories: (.value.repositories // ""), + packages: (.value.packages // []), + ("has-app"): "true" + }))) ] | add // []') count=$(echo "$groups" | jq 'length') @@ -242,7 +256,9 @@ jobs: fi done < <(echo "$groups" | jq -r '.[].id') - # SAFE: emit only id + package-count to logs. Never $groups in full. + # Emit only id + package-count to logs to keep notice output tight. + # The matrix itself is non-secret under the env-relay design, but a + # condensed summary remains easier to scan in the run UI. { echo "matrix={\"group\":$groups}" } >> "$GITHUB_OUTPUT" @@ -256,14 +272,78 @@ jobs: strategy: fail-fast: false matrix: ${{ fromJSON(needs.apm-prep.outputs.matrix) }} + env: + # gh-aw text-substitutes these at compile time. They land in the + # apm job's per-replica env, which the runner masks in logs but + # does NOT redact-strip the way job outputs are. From here the + # "Resolve credentials" step picks the right row by matrix.group + # routing metadata and relays only the resolved values into + # $GITHUB_ENV. This is the workaround for GitHub Actions silently + # dropping job outputs whose value contains a registered secret + # substring (HostContext.SecretMasker output filter). + AW_APM_LEGACY_APP_ID: ${{ github.aw.import-inputs.app-id }} + AW_APM_LEGACY_PRIVATE_KEY: ${{ github.aw.import-inputs.private-key }} + AW_APM_APPS: ${{ github.aw.import-inputs.apps }} steps: + - name: Resolve credentials for this matrix row + if: ${{ matrix.group.has-app == 'true' }} + env: + ROW_KIND: ${{ matrix.group.kind }} + ROW_INDEX: ${{ matrix.group.index }} + run: | + # SECURITY: never `set -x` or `echo "$pk"` in this step. ::add-mask:: + # registers the PEM as a single multi-line substring; the masker will + # not match individual PEM lines printed in isolation, so any future + # debug echo of $pk line-by-line would leak the key body in clear text. + set -euo pipefail + case "$ROW_KIND" in + legacy) + app_id="${AW_APM_LEGACY_APP_ID:-}" + pk="${AW_APM_LEGACY_PRIVATE_KEY:-}" + ;; + apps) + apps_json="${AW_APM_APPS:-[]}" + app_id=$(printf '%s' "$apps_json" | jq -r --argjson i "$ROW_INDEX" '.[$i]["app-id"] // ""') + pk=$(printf '%s' "$apps_json" | jq -r --argjson i "$ROW_INDEX" '.[$i]["private-key"] // ""') + ;; + *) + echo "::error::unexpected apm matrix kind '$ROW_KIND' for row with has-app=true" + exit 1 + ;; + esac + if [ -z "$app_id" ] || [ -z "$pk" ]; then + echo "::error::missing app-id or private-key for apm row kind=$ROW_KIND index=$ROW_INDEX" + exit 1 + fi + # Normalise trailing newline. Bash $(jq ...) strips ALL trailing + # newlines from PEMs read out of the apps[] JSON, while a direct + # env-var assignment preserves them. Stripping any tail and adding + # exactly one makes the legacy and apps paths produce byte-identical + # ROW_PRIVATE_KEY values so downstream tolerance is irrelevant. + pk="${pk%$'\n'}" + # Defence in depth: the PK is already masked because it came from + # a ${{ secrets.* }} reference at compile time, but registering it + # again here makes the contract explicit and survives any future + # gh-aw template churn that might lose the secret tag. + echo "::add-mask::$pk" + # Use a random heredoc delimiter to eliminate any chance of a PEM + # line collision terminating the value early. The official docs + # explicitly warn against fixed delimiters for arbitrary multi-line + # values: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings + delim="APMPK_$(openssl rand -hex 16)" + { + echo "ROW_APP_ID=$app_id" + printf 'ROW_PRIVATE_KEY<<%s\n' "$delim" + printf '%s\n' "$pk" + printf '%s\n' "$delim" + } >> "$GITHUB_ENV" - name: Mint installation token id: token - if: ${{ matrix.group.app-id != '' }} + if: ${{ matrix.group.has-app == 'true' }} uses: actions/create-github-app-token@v3.1.1 with: - app-id: ${{ matrix.group.app-id }} - private-key: ${{ matrix.group.private-key }} + app-id: ${{ env.ROW_APP_ID }} + private-key: ${{ env.ROW_PRIVATE_KEY }} owner: ${{ matrix.group.owner != '' && matrix.group.owner || github.repository_owner }} repositories: ${{ matrix.group.repositories }} - name: Render package list diff --git a/.github/workflows/verify-shared-apm-matrix.yml b/.github/workflows/verify-shared-apm-matrix.yml new file mode 100644 index 000000000..7ef449d9a --- /dev/null +++ b/.github/workflows/verify-shared-apm-matrix.yml @@ -0,0 +1,283 @@ +name: Verify shared/apm.md matrix secret-stripping fix + +# Empirical proof for the env-relay fix in .github/workflows/shared/apm.md. +# +# Two job sets: +# +# A. prove-old-pattern-strips-output (regression sentinel) +# Replicates the pre-fix shape (PEM embedded in a job output). Asserts +# that GitHub Actions still drops the output via its secret-redaction +# filter (HostContext.SecretMasker.MaskSecrets in actions/runner). If +# this job ever fails, GH has loosened the rule and we should reassess +# the fix. +# +# B. prove-new-pattern-survives (positive proof of the fix) +# Replicates the post-fix shape (secret-free matrix + env-relay creds). +# Asserts: (1) the matrix output survives, (2) the dependent matrix job +# fans out to the expected number of rows, (3) each row successfully +# resolves ROW_APP_ID / ROW_PRIVATE_KEY via $GITHUB_ENV, and the PEM +# round-trips intact (SHA-256 comparison; the SHA is not a substring +# of the PEM and is therefore not masked). +# +# Self-contained: no real GitHub-App credentials required. The synthetic +# PEM below is registered at runtime via ::add-mask:: which feeds the same +# HostContext.SecretMasker that processes repo / org secrets. + +on: + pull_request: + paths: + - .github/workflows/shared/apm.md + - .github/workflows/verify-shared-apm-matrix.yml + push: + branches: + - main + paths: + - .github/workflows/shared/apm.md + - .github/workflows/verify-shared-apm-matrix.yml + workflow_dispatch: + +permissions: + contents: read + +env: + # Synthetic, non-functional PEM used in both job sets. Truncated key body + # so it cannot accidentally be used to authenticate anywhere. Both job + # sets register it via ::add-mask:: so the runner treats it as a secret. + SYNTHETIC_PEM: | + -----BEGIN RSA PRIVATE KEY----- + MIIBOgIBAAJBAKj34GkxFhD90vcNLYLInFEX6Ppy1tPf9Cnzj4p4WGeKLs1Pt8Qu + SYNTHETIC-TEST-KEY-DO-NOT-USE + -----END RSA PRIVATE KEY----- + +jobs: + # ===================================================================== + # Job set A: regression sentinel. + # ===================================================================== + a-bug-prep: + name: A. Old pattern (broken) - prep + runs-on: ubuntu-latest + outputs: + matrix: ${{ steps.emit.outputs.matrix }} + steps: + - name: Register synthetic PEM as a job secret + id: pem + run: | + set -euo pipefail + # ::add-mask:: registers the value in this job's SecretMasker. + # Any subsequent step output OR job output containing this + # substring will be subject to the runner's masking / stripping. + while IFS= read -r line; do + if [ -n "$line" ]; then echo "::add-mask::$line"; fi + done <<< "$SYNTHETIC_PEM" + # Pass through as a step output (step outputs are NOT subject + # to the cross-job redaction filter; only job outputs are). + { + echo "pem<> "$GITHUB_OUTPUT" + - name: Emit broken-shape matrix (PEM embedded in job output) + id: emit + env: + PEM: ${{ steps.pem.outputs.pem }} + run: | + set -euo pipefail + # This is exactly the shape the pre-fix shared/apm.md emitted: + # a matrix JSON whose "private-key" value contains the PEM. + matrix=$(jq -nc --arg pk "$PEM" '{group:[{id:"x","private-key":$pk}]}') + { + echo "matrix=$matrix" + } >> "$GITHUB_OUTPUT" + # Note: we never echo $matrix to logs (masking would obscure it + # anyway). The downstream job will receive an empty string here. + + a-bug-consume: + name: A. Old pattern (broken) - assert stripped + runs-on: ubuntu-latest + needs: [a-bug-prep] + steps: + - name: Assert matrix output was stripped by GH secret filter + env: + GOT: ${{ needs.a-bug-prep.outputs.matrix }} + run: | + set -euo pipefail + # GitHub Actions runs every job output through + # if (!string.Equals(out.Value, SecretMasker.MaskSecrets(out.Value))) + # context.Warning("Skip output '' since it may contain secret."); + # When the warning fires, the output is replaced with the empty + # string before downstream jobs see it. We assert that holds. + if [ -n "$GOT" ]; then + echo "::error::Expected matrix output to be empty (stripped by GH redaction filter)." + echo "::error::Got non-empty value of length ${#GOT}. Either GH changed the behaviour, or the synthetic PEM was not masked." + exit 1 + fi + echo "[OK] matrix output is empty -- GH redaction filter still active." + echo "[OK] this proves shared/apm.md's pre-fix shape would silently break consumers." + + # ===================================================================== + # Job set B: positive proof that the env-relay fix survives. + # ===================================================================== + b-fix-prep: + name: B. New pattern (fixed) - prep + runs-on: ubuntu-latest + outputs: + matrix: ${{ steps.emit.outputs.matrix }} + expected_sha: ${{ steps.emit.outputs.expected_sha }} + steps: + - name: Register synthetic PEM as a job secret (proves output survives anyway) + run: | + set -euo pipefail + # We mask the PEM in this job too, so the redaction filter is + # primed. The point is: even with the PEM registered, our + # secret-FREE matrix output must still survive end-of-job + # masking because the JSON value does not contain the PEM + # substring. This is the core property the fix relies on. + while IFS= read -r line; do + if [ -n "$line" ]; then echo "::add-mask::$line"; fi + done <<< "$SYNTHETIC_PEM" + - name: Emit secret-free matrix + non-secret expected_sha + id: emit + run: | + set -euo pipefail + # Two rows mirroring the production schema: one "legacy" group, + # one "apps[]" group. No PEM, no app-id; only routing metadata. + matrix=$(jq -nc '{group:[ + {id:"legacy", kind:"legacy", index:0, "has-app":"true", owner:"", repositories:"", packages:[]}, + {id:"acme-app", kind:"apps", index:0, "has-app":"true", owner:"acme", repositories:"", packages:[]} + ]}') + # SHA-256 of the PEM, computed AFTER stripping trailing newlines + # so the legacy and apps[] rows in b-fix-consume can converge on + # the same hash regardless of whether their resolution path went + # through bash command substitution (which strips trailing \n) + # or direct env-var read (which preserves them). A SHA is not a + # substring of the PEM, so the masker leaves it alone. + pem_trim="$SYNTHETIC_PEM" + while [ "${pem_trim: -1}" = $'\n' ]; do pem_trim=${pem_trim%$'\n'}; done + sha=$(printf '%s' "$pem_trim" | sha256sum | cut -d' ' -f1) + { + echo "matrix=$matrix" + echo "expected_sha=$sha" + } >> "$GITHUB_OUTPUT" + echo "matrix rows: $(echo "$matrix" | jq '.group | length')" + echo "expected_sha: $sha" + + b-fix-assert-shape: + name: B. New pattern (fixed) - assert matrix survives + runs-on: ubuntu-latest + needs: [b-fix-prep] + steps: + - name: Assert matrix output non-empty and has expected rows + env: + GOT: ${{ needs.b-fix-prep.outputs.matrix }} + GOT_SHA: ${{ needs.b-fix-prep.outputs.expected_sha }} + run: | + set -euo pipefail + if [ -z "$GOT" ]; then + echo "::error::matrix output is EMPTY -- the secret-free shape should NOT be stripped." + echo "::error::Either the PEM substring leaked into the matrix JSON, or GH tightened the filter." + exit 1 + fi + count=$(printf '%s' "$GOT" | jq '.group | length') + if [ "$count" != "2" ]; then + echo "::error::expected 2 matrix rows, got $count" + exit 1 + fi + if [ -z "$GOT_SHA" ] || [ "${#GOT_SHA}" -ne 64 ]; then + echo "::error::expected_sha output missing or wrong length: '$GOT_SHA'" + exit 1 + fi + echo "[OK] matrix output survived (length ${#GOT}, $count rows)." + echo "[OK] expected_sha output survived ($GOT_SHA)." + + b-fix-consume: + name: B. New pattern (fixed) - per-row env-relay + runs-on: ubuntu-latest + needs: [b-fix-prep] + strategy: + fail-fast: false + matrix: ${{ fromJSON(needs.b-fix-prep.outputs.matrix) }} + steps: + - name: Re-register synthetic PEM as a job secret for this matrix row + run: | + set -euo pipefail + while IFS= read -r line; do + if [ -n "$line" ]; then echo "::add-mask::$line"; fi + done <<< "$SYNTHETIC_PEM" + - name: Resolve credentials for this matrix row (env-relay) + env: + # In production shared/apm.md these are substituted by gh-aw at + # compile time from ${{ github.aw.import-inputs.* }}, set on + # the apm job's env: block. Here we set them at step level so + # we can reference the workflow-level env.SYNTHETIC_PEM (env + # context is allowed in step-level env but not job-level env). + AW_APM_LEGACY_APP_ID: '12345' + AW_APM_LEGACY_PRIVATE_KEY: ${{ env.SYNTHETIC_PEM }} + ROW_KIND: ${{ matrix.group.kind }} + ROW_INDEX: ${{ matrix.group.index }} + run: | + # SECURITY: mirrors the production shared/apm.md hardenings. + # Never `set -x` or echo $pk; ::add-mask:: registers the whole PEM + # as one substring and will not mask individual lines. + set -euo pipefail + case "$ROW_KIND" in + legacy) + app_id="$AW_APM_LEGACY_APP_ID" + pk="$AW_APM_LEGACY_PRIVATE_KEY" + ;; + apps) + # Build the apps[] JSON inline with the synthetic PEM at + # the indexed slot; mirrors what jq -r ".[$i]" does over + # the substituted AW_APM_APPS in production shared/apm.md. + apps_json=$(jq -nc --arg pk "$AW_APM_LEGACY_PRIVATE_KEY" '[{"app-id":"99999","private-key":$pk}]') + app_id=$(printf '%s' "$apps_json" | jq -r --argjson i "$ROW_INDEX" '.[$i]["app-id"] // ""') + pk=$(printf '%s' "$apps_json" | jq -r --argjson i "$ROW_INDEX" '.[$i]["private-key"] // ""') + ;; + *) + echo "::error::unexpected kind '$ROW_KIND'" + exit 1 + ;; + esac + if [ -z "$app_id" ] || [ -z "$pk" ]; then + echo "::error::failed to resolve credentials for row kind=$ROW_KIND index=$ROW_INDEX" + exit 1 + fi + # Normalise trailing newline so legacy (direct env) and apps + # (jq + $(...) which strips trailing \n) produce byte-identical + # ROW_PRIVATE_KEY values, removing the asymmetry at the source. + pk="${pk%$'\n'}" + echo "::add-mask::$pk" + # Random heredoc delimiter per https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings + # which warns against fixed delimiters for arbitrary multi-line values. + delim="APMPK_$(openssl rand -hex 16)" + { + echo "ROW_APP_ID=$app_id" + printf 'ROW_PRIVATE_KEY<<%s\n' "$delim" + printf '%s\n' "$pk" + printf '%s\n' "$delim" + } >> "$GITHUB_ENV" + - name: Assert ROW_PRIVATE_KEY round-tripped intact + env: + EXPECTED_SHA: ${{ needs.b-fix-prep.outputs.expected_sha }} + ROW_KIND: ${{ matrix.group.kind }} + run: | + set -euo pipefail + if [ -z "${ROW_APP_ID:-}" ]; then + echo "::error::ROW_APP_ID was not set by the resolver step" + exit 1 + fi + if [ -z "${ROW_PRIVATE_KEY:-}" ]; then + echo "::error::ROW_PRIVATE_KEY was not set by the resolver step" + exit 1 + fi + # No normalisation needed here: the resolver now strips trailing + # newlines uniformly before the heredoc, and the $GITHUB_ENV + # heredoc parser strips exactly one trailing \n before the + # delimiter (per FileCommandManager). Both paths converge on the + # same byte sequence -- assert directly. + got_sha=$(printf '%s' "$ROW_PRIVATE_KEY" | sha256sum | cut -d' ' -f1) + if [ "$got_sha" != "$EXPECTED_SHA" ]; then + echo "::error::ROW_PRIVATE_KEY SHA mismatch (got=$got_sha expected=$EXPECTED_SHA)" + echo "::error::The env-relay corrupted the PEM body. Check newline/heredoc handling." + exit 1 + fi + echo "[OK] row kind=$ROW_KIND app_id=$ROW_APP_ID pk_sha=$got_sha matches expected." diff --git a/CHANGELOG.md b/CHANGELOG.md index fc46e3204..0a7581cba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed `shared/apm.md` matrix fan-out being silently stripped by the GitHub Actions cross-job secret masker when the GitHub App `private-key` was carried in the `apm-prep` matrix output; credentials are now resolved per row via `$GITHUB_ENV` in the `apm` job. (#1373) - `apm compile --watch` now honors `targets: [claude, cursor]` (and every other multi-target / single-target configuration) on every recompile. Previously the watch path bypassed the resolver the one-shot path uses and let `CompilationConfig.from_apm_yml` fall back to the all-families default, silently regenerating `GEMINI.md` after every file edit. The watch path now resolves the effective target via the same helper the one-shot path uses and forwards it as `target=` into both the initial compile and every debounced recompile. (#1345) - Fixed hook commands using relative paths that break for Claude target (#1310) - Fixed target not propagating to intermediate CompilationConfig during compilation (#765)