From 73c164c1fdebf81e1e7363e6cb48324ca7af9c1b Mon Sep 17 00:00:00 2001 From: Daniel Meppiel Date: Mon, 18 May 2026 22:17:14 +0200 Subject: [PATCH 1/5] Fix shared/apm.md matrix secret-stripping; add verify workflow The pre-fix shared/apm.md embedded the GitHub App private-key inside the apm-prep job's 'matrix' output. GitHub Actions ran every job output through HostContext.SecretMasker.MaskSecrets (see actions/runner JobExtension.cs); when the masked value differed from the original, the runner dropped the output entirely with the warning 'Skip output "matrix" since it may contain secret.' The downstream apm job's matrix: fromJSON(needs.apm-prep.outputs.matrix) then received an empty string, no replicas ran, no APM bundles were packed, and pre-agent-steps failed at 'no apm bundles found'. Every single-app legacy (form 2) and apps[] (form 3) consumer of shared/apm.md was broken end-to-end. The fix moves credentials off the job-output path. apm-prep now emits routing metadata only (id, kind, index, owner, repositories, packages, has-app); the apm job carries the gh-aw-substituted secrets in its own per-replica env, and a new 'Resolve credentials' step picks the row's credentials with jq and relays them through $GITHUB_ENV as ROW_APP_ID / ROW_PRIVATE_KEY. The create-github-app-token step now reads from env.ROW_*. $GITHUB_ENV is per-job and is masked-in-logs but NOT subject to the cross-job output redaction filter. Empirical proof: .github/workflows/verify-shared-apm-matrix.yml exercises both shapes with a synthetic ::add-mask::'d PEM (no real secrets needed). Job set A replicates the pre-fix shape and asserts the matrix output IS stripped (regression sentinel against GH silently changing the rule). Job set B replicates the post-fix shape and asserts the matrix output survives, the matrix-strategy job fans out to two rows (legacy + apps), each row resolves ROW_PRIVATE_KEY from $GITHUB_ENV, and the PEM round-trips intact via SHA-256 comparison. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/shared/apm.md | 87 +++++- .../workflows/verify-shared-apm-matrix.yml | 266 ++++++++++++++++++ CHANGELOG.md | 1 + 3 files changed, 343 insertions(+), 11 deletions(-) create mode 100644 .github/workflows/verify-shared-apm-matrix.yml diff --git a/.github/workflows/shared/apm.md b/.github/workflows/shared/apm.md index 4b5438153..7f8b93804 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,63 @@ 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: | + 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 + # 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" + { + echo "ROW_APP_ID=$app_id" + echo "ROW_PRIVATE_KEY<> "$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..419cd78f5 --- /dev/null +++ b/.github/workflows/verify-shared-apm-matrix.yml @@ -0,0 +1,266 @@ +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 + [ -n "$line" ] && echo "::add-mask::$line" + 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 + [ -n "$line" ] && echo "::add-mask::$line" + 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. A SHA is not a substring of the PEM, so + # the masker leaves it alone. Downstream rows recompute the + # SHA of their resolved ROW_PRIVATE_KEY and compare. + sha=$(printf '%s' "$SYNTHETIC_PEM" | 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 + [ -n "$line" ] && echo "::add-mask::$line" + 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: | + 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 + # Defence in depth: re-mask the resolved PK before writing it + # to $GITHUB_ENV. The masker is run-scoped per-job, so this is + # mostly belt-and-suspenders, but matches production shape. + echo "::add-mask::$pk" + { + echo "ROW_APP_ID=$app_id" + echo "ROW_PRIVATE_KEY<> "$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 + 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..b68d429a4 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 +- `shared/apm.md`: rebuilt the apm-prep -> apm matrix hand-off so the per-row credentials are relayed via `$GITHUB_ENV` on the `apm` job instead of being embedded in the matrix job output. The previous design placed the GitHub App `private-key` value inside the `matrix` job output of `apm-prep`, which the runner stripped via its cross-job secret-redaction filter (`HostContext.SecretMasker.MaskSecrets` in `actions/runner` / `JobExtension.cs`, surfaced as `##[warning]Skip output 'matrix' since it may contain secret.`). Downstream consumers received an empty matrix, zero `apm` replicas ran, and pre-agent-steps failed at `no apm bundles found`. Every form 2 (single-app legacy) and form 3 (`apps[]`) consumer of `shared/apm.md` was broken end-to-end. The fix moves credentials out of the matrix output entirely: `apm-prep` now emits routing metadata only (`id`, `kind`, `index`, `owner`, `repositories`, `packages`, `has-app`); the `apm` job carries the gh-aw-substituted secrets in its own per-replica env, resolves the row credentials via `jq`, and relays them to `create-github-app-token` through `ROW_APP_ID` / `ROW_PRIVATE_KEY` in `$GITHUB_ENV` (which is per-job and never subject to the cross-job redaction filter). A new `.github/workflows/verify-shared-apm-matrix.yml` workflow provides empirical proof: one job set replicates the broken pre-fix shape and asserts the matrix output is stripped (regression sentinel for the runner behaviour), and a second job set replicates the post-fix shape and asserts the matrix output survives, the `apm`-style matrix job fans out, and the resolved `ROW_PRIVATE_KEY` round-trips intact (SHA-256 comparison). - `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) From 7c632d277adc84035bb6967d0be33417c10668a2 Mon Sep 17 00:00:00 2001 From: Daniel Meppiel Date: Mon, 18 May 2026 22:21:03 +0200 Subject: [PATCH 2/5] verify-shared-apm-matrix: avoid set -e exit on empty PEM line The loop body '[ -n "$line" ] && echo ...' returns the exit status of the failed test when $line is empty, which under 'set -e' aborts the script. Use an explicit 'if ...; then ...; fi' instead so the loop tolerates the trailing empty line from the heredoc-string read deterministically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/verify-shared-apm-matrix.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/verify-shared-apm-matrix.yml b/.github/workflows/verify-shared-apm-matrix.yml index 419cd78f5..f7f33df87 100644 --- a/.github/workflows/verify-shared-apm-matrix.yml +++ b/.github/workflows/verify-shared-apm-matrix.yml @@ -67,7 +67,7 @@ jobs: # 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 - [ -n "$line" ] && echo "::add-mask::$line" + 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). @@ -133,7 +133,7 @@ jobs: # 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 - [ -n "$line" ] && echo "::add-mask::$line" + if [ -n "$line" ]; then echo "::add-mask::$line"; fi done <<< "$SYNTHETIC_PEM" - name: Emit secret-free matrix + non-secret expected_sha id: emit @@ -196,7 +196,7 @@ jobs: run: | set -euo pipefail while IFS= read -r line; do - [ -n "$line" ] && echo "::add-mask::$line" + if [ -n "$line" ]; then echo "::add-mask::$line"; fi done <<< "$SYNTHETIC_PEM" - name: Resolve credentials for this matrix row (env-relay) env: From 46143432c63b3fdcc7c9434cf322fb8b1056f5c7 Mon Sep 17 00:00:00 2001 From: Daniel Meppiel Date: Mon, 18 May 2026 22:23:35 +0200 Subject: [PATCH 3/5] verify-shared-apm-matrix: normalise trailing newlines before SHA The legacy resolution path preserves the PEM's trailing newline (direct env-var expansion), while the apps[] path strips it ($() command substitution around jq -r). Both are equally valid for a real PEM that downstream create-github-app-token consumes, but the bit-exact SHA-256 comparison would diverge by one newline. Trim trailing newlines on both the expected (prep) and got (consume) side before hashing so both rows converge on the same value, without weakening the substring-leak detection (the trim affects only whitespace, not PEM body bytes). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../workflows/verify-shared-apm-matrix.yml | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/.github/workflows/verify-shared-apm-matrix.yml b/.github/workflows/verify-shared-apm-matrix.yml index f7f33df87..37b3d2283 100644 --- a/.github/workflows/verify-shared-apm-matrix.yml +++ b/.github/workflows/verify-shared-apm-matrix.yml @@ -145,10 +145,15 @@ jobs: {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. A SHA is not a substring of the PEM, so - # the masker leaves it alone. Downstream rows recompute the - # SHA of their resolved ROW_PRIVATE_KEY and compare. - sha=$(printf '%s' "$SYNTHETIC_PEM" | sha256sum | cut -d' ' -f1) + # 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" @@ -257,7 +262,13 @@ jobs: echo "::error::ROW_PRIVATE_KEY was not set by the resolver step" exit 1 fi - got_sha=$(printf '%s' "$ROW_PRIVATE_KEY" | sha256sum | cut -d' ' -f1) + # Normalise trailing newlines before hashing so both resolution + # paths (direct env-var read for legacy preserves trailing \n; + # jq + command substitution for apps strips it) converge on the + # same SHA. Matches the normalisation in b-fix-prep. + pk_trim="$ROW_PRIVATE_KEY" + while [ "${pk_trim: -1}" = $'\n' ]; do pk_trim=${pk_trim%$'\n'}; done + got_sha=$(printf '%s' "$pk_trim" | 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." From e027211af6dc9ebea079151b3c3140d2eb79556b Mon Sep 17 00:00:00 2001 From: Daniel Meppiel Date: Mon, 18 May 2026 22:49:21 +0200 Subject: [PATCH 4/5] harden(apm.md): apply 3-expert audit findings to resolver Three parallel adversarial expert audits (secret-masker, GitHub Apps, matrix-composition) returned MOSTLY CONFIDENT on the env-relay fix and converged on three concrete hardenings worth applying. Mirrors the production resolver in the verify workflow so the empirical proof exercises the hardened code path. 1) Random heredoc delimiter (matrix-composition expert) Replace the fixed 'APMPK' delimiter with 'APMPK_$(openssl rand -hex 16)'. The official multi-line workflow command docs explicitly warn against fixed delimiters for arbitrary multi-line values; the runner's FileCommandManager does an exact-line ordinal compare, so a PEM whose base64 body happened to contain a line equal to 'APMPK' would prematurely terminate the value. Probability for real PEMs is astronomically low but non-zero; eliminate the class entirely. 2) Trailing-newline normalisation (matrix-composition expert) The legacy path (direct env var) preserves the PEM's trailing newline while the apps[] path (jq via $(...) command substitution) strips it. create-github-app-token v3.1.1 tolerates both via core.getInput().trim(), but normalising once at the env-relay boundary removes the asymmetry at the source instead of leaning on downstream tolerance. Both paths now produce byte-identical ROW_PRIVATE_KEY values. 3) Never-echo SECURITY comment (secret-masker + github-apps experts) ::add-mask:: registers the PEM as a single multi-line substring; the runner's secret masker does not match individual PEM lines printed in isolation. A future debug 'echo $pk' or 'set -x' in this step would leak the key body in clear text. Pure preventive guard. Primary sources cited by the audits: - actions/runner JobExtension.cs:639-647 (the output filter itself) - actions/runner FileCommandManager.cs:378 (heredoc parser - exact compare) - actions/runner ActionCommandManager.cs:426 (::add-mask:: implementation) - docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings - raw.githubusercontent.com/actions/create-github-app-token/v3.1.1/main.js Verify workflow simplification: with the resolver now normalising trailing newlines uniformly, the b-fix-consume SHA assertion no longer needs its own normalisation loop -- both rows arrive at byte-identical PEMs at the $GITHUB_ENV boundary. Drops 4 lines of belt-and-suspenders. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/shared/apm.md | 19 ++++++++++-- .../workflows/verify-shared-apm-matrix.yml | 30 +++++++++++-------- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/.github/workflows/shared/apm.md b/.github/workflows/shared/apm.md index 7f8b93804..47194cd1d 100644 --- a/.github/workflows/shared/apm.md +++ b/.github/workflows/shared/apm.md @@ -291,6 +291,10 @@ jobs: 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) @@ -311,16 +315,27 @@ jobs: 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" - echo "ROW_PRIVATE_KEY<> "$GITHUB_ENV" - name: Mint installation token id: token diff --git a/.github/workflows/verify-shared-apm-matrix.yml b/.github/workflows/verify-shared-apm-matrix.yml index 37b3d2283..7ef449d9a 100644 --- a/.github/workflows/verify-shared-apm-matrix.yml +++ b/.github/workflows/verify-shared-apm-matrix.yml @@ -215,6 +215,9 @@ jobs: 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) @@ -238,15 +241,19 @@ jobs: echo "::error::failed to resolve credentials for row kind=$ROW_KIND index=$ROW_INDEX" exit 1 fi - # Defence in depth: re-mask the resolved PK before writing it - # to $GITHUB_ENV. The masker is run-scoped per-job, so this is - # mostly belt-and-suspenders, but matches production shape. + # 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" - echo "ROW_PRIVATE_KEY<> "$GITHUB_ENV" - name: Assert ROW_PRIVATE_KEY round-tripped intact env: @@ -262,13 +269,12 @@ jobs: echo "::error::ROW_PRIVATE_KEY was not set by the resolver step" exit 1 fi - # Normalise trailing newlines before hashing so both resolution - # paths (direct env-var read for legacy preserves trailing \n; - # jq + command substitution for apps strips it) converge on the - # same SHA. Matches the normalisation in b-fix-prep. - pk_trim="$ROW_PRIVATE_KEY" - while [ "${pk_trim: -1}" = $'\n' ]; do pk_trim=${pk_trim%$'\n'}; done - got_sha=$(printf '%s' "$pk_trim" | sha256sum | cut -d' ' -f1) + # 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." From 65e58684122ab3de852b72b9987bb842476d548a Mon Sep 17 00:00:00 2001 From: Daniel Meppiel Date: Mon, 18 May 2026 22:54:14 +0200 Subject: [PATCH 5/5] changelog: address review feedback - compress to one-line entry with PR ref Per review comment from copilot-pull-request-reviewer on PR #1373: - Add missing (#1373) suffix to match Keep a Changelog format used throughout the file. - Compress the multi-paragraph root-cause exposition to a single line; the deep technical detail (runner internals, JobExtension.cs, verify workflow design) lives in the PR description where it belongs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b68d429a4..0a7581cba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- `shared/apm.md`: rebuilt the apm-prep -> apm matrix hand-off so the per-row credentials are relayed via `$GITHUB_ENV` on the `apm` job instead of being embedded in the matrix job output. The previous design placed the GitHub App `private-key` value inside the `matrix` job output of `apm-prep`, which the runner stripped via its cross-job secret-redaction filter (`HostContext.SecretMasker.MaskSecrets` in `actions/runner` / `JobExtension.cs`, surfaced as `##[warning]Skip output 'matrix' since it may contain secret.`). Downstream consumers received an empty matrix, zero `apm` replicas ran, and pre-agent-steps failed at `no apm bundles found`. Every form 2 (single-app legacy) and form 3 (`apps[]`) consumer of `shared/apm.md` was broken end-to-end. The fix moves credentials out of the matrix output entirely: `apm-prep` now emits routing metadata only (`id`, `kind`, `index`, `owner`, `repositories`, `packages`, `has-app`); the `apm` job carries the gh-aw-substituted secrets in its own per-replica env, resolves the row credentials via `jq`, and relays them to `create-github-app-token` through `ROW_APP_ID` / `ROW_PRIVATE_KEY` in `$GITHUB_ENV` (which is per-job and never subject to the cross-job redaction filter). A new `.github/workflows/verify-shared-apm-matrix.yml` workflow provides empirical proof: one job set replicates the broken pre-fix shape and asserts the matrix output is stripped (regression sentinel for the runner behaviour), and a second job set replicates the post-fix shape and asserts the matrix output survives, the `apm`-style matrix job fans out, and the resolved `ROW_PRIVATE_KEY` round-trips intact (SHA-256 comparison). +- 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)