ci(trailers): enforce agent trailer preservation at commit, merge, and CI layers

[steilerDev]

Summary

• Layer 0 (root cause): dev-team-lead [MODE: commit] now self-derives the required trailer set from the staged-file diff against CLAUDE.md's Delegation Enforcement rules instead of trusting the orchestrator's contributing-agents list verbatim — the dominant failure mode found in Trailer verification not enforced: 51% of production commits missing required agent trailers; format drift; direct push to beta #1820 was single-commit PRs where the original commit already omitted a trailer, which no merge-time fix can repair after the fact.
• Layer 1 (defense in depth): every skill that squash-merges a PR (develop, epic-close, dependabot, fix-e2e, release) now rebuilds the squash body explicitly via a new canonical CLAUDE.md pattern (union of commit trailers, case-normalized, deduped) instead of relying on GitHub's default squash-body behavior, which is lossy for multi-commit branches.
• Layer 2 (automated backstop): new required trailer-check CI job (scripts/check-trailers.sh) validates trailer presence against the same 6 ownership rules on every PR touching production paths — required because the existing manual verification step (/develop step 6h) was not preventing the violations it was designed to catch.
• fix-e2e (which commits directly, bypassing dev-team-lead) now uses the exact canonical trailer string instead of a generic placeholder, closing the format-drift gap that produced non-canonical trailers.

Fixes #1820

Test plan

• scripts/check-trailers.sh verified against known-bad (e90d68b5^..e90d68b5 → exit 1, 4 missing-trailer errors), known-good (936dda16 → exit 0), and human-only-authored range (exit 0, skip)
• bash -n scripts/check-trailers.sh syntax check passes; executable bit (100755) confirmed
• ci.yml YAML changes reviewed: trailer-check wired into quality-gates needs + check_job, using the same skip-tolerant pattern as the other conditionally-skipped jobs
• Reviewed by dev-team-lead in [MODE: review] — VERDICT: APPROVED, no changes required
• CI Quality Gates pass

Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) noreply@anthropic.com

[steilerDev]

[security-engineer] Security review of #1852 (fix/1820-trailer-enforcement → beta).

Scope covered: new trailer-check CI job (.github/workflows/ci.yml), new scripts/check-trailers.sh, the Squash-Merge Trailer Preservation pattern added to CLAUDE.md and replicated across 5 skills, and the dev-team-lead.md self-derivation logic. This is process/governance tooling (commit attribution enforcement), not application code, so the attack surface is narrow — CI script injection and shell command injection were the two angles worth chasing.

Verdict: APPROVED

---

Findings

[Informational] github.base_ref interpolated directly into a run: shell block

• .github/workflows/ci.yml:86 — BASE_REF="${{ github.base_ref }}" inside the new trailer-check job's run: step.
• This is the textbook shape of the GitHub Actions "script injection" anti-pattern (untrusted ${{ }} expression substituted directly into shell source before bash ever parses it, as opposed to being passed through as an inert environment variable). GitHub's own Security Hardening Guide and CodeQL's actions/script-injection query both flag this construct on principle.
• However, exploitability here is low. Unlike github.head_ref (attacker-chosen branch name in a fork, well-documented as untrusted), github.base_ref names a branch that must already exist in the base repository (steilerDev/cornerstone). An external fork contributor selects an existing base branch when opening a PR — they cannot invent an arbitrary base branch name containing shell metacharacters, since that would require push access to the upstream repo (at which point far more direct attacks are already available). GitHub's hardening guide does not list base_ref among the untrusted-input fields for this reason.
• Not a new pattern introduced by this PR — it's a direct copy of the pre-existing detect-changes job at ci.yml:27, which predates this PR. I'm not treating this as a regression, but the copy-paste is worth breaking now rather than propagating further.
• Remediation (defense-in-depth, addresses both the new and pre-existing occurrence together in a follow-up):

  - name: Verify agent trailers
    env:
      BASE_REF: ${{ github.base_ref }}
    run: |
      BASE_REF="${BASE_REF:-main}"
      bash scripts/check-trailers.sh "origin/${BASE_REF}" "HEAD"

  Routing the expression through env: means bash only ever sees a plain variable value, never a literal template substitution — so the pattern stays safe even if someone later copies it onto a genuinely attacker-controlled field (head_ref, PR title, issue title, etc.).
• Not blocking. Suggest a small follow-up PR to fix both occurrences (ci.yml:27 and ci.yml:86) together.

[Informational] No explicit permissions: block on the new job

• trailer-check doesn't declare a permissions: block scoping GITHUB_TOKEN to read-only. This matches the rest of the workflow (no job in ci.yml sets explicit permissions), so it's pre-existing repo posture, not something this PR regresses. The job doesn't call gh, doesn't write artifacts, and doesn't touch secrets, so there's nothing for excess token scope to leverage today — flagging only as a standing hardening opportunity, not specific to this PR.

Confirmed safe (no finding)

• scripts/check-trailers.sh: set -euo pipefail present; all variable expansions quoted ("${BASE_REF}", "${CHANGED}", etc.); no eval; regex matching via grep -E never re-executes matched text. Skip condition (no Claude trailers in range → exit 0) is a governance/attribution check, not an authorization gate — bypassing it doesn't let malicious code through, it just means human-authored commits aren't required to carry agent trailers, which is the intended behavior.
• Squash-body heredoc pattern (CLAUDE.md "Squash-Merge Trailer Preservation", replicated in develop, epic-close, fix-e2e, dependabot, release skills): TRAILERS=$(git log ... | grep ...) followed by ${TRAILERS} interpolated into an unquoted <<EOF heredoc. I traced this for double-expansion risk (a maliciously crafted commit trailer containing $(...) or backticks somehow surviving into executed shell) — bash does not recursively re-scan the result of a parameter expansion for further command/command substitution; expansion is single-pass. A trailer line containing shell metacharacters would be inserted as an inert literal string into $BODY, then passed as a single quoted argument to gh pr merge --body "$BODY" — no re-interpretation occurs. Also relevant as a practical (not just mechanical) mitigant: these commits are written by the orchestrator/dev-team-lead on branches only agents commit to in this project's workflow, not by external/fork contributors, so the trust boundary is sound even before the mechanical analysis.
• CI job wiring: needs: [detect-changes], fetch-depth: 0 (required for the git log/git diff range comparisons, not a secret-exposure concern), pinned actions/checkout@9c091bb2... SHA consistent with the rest of the file. No secrets referenced anywhere in the new job or script.
• Dependabot skip condition (github.event.pull_request.user.login != 'dependabot[bot]'): mirrors the existing pattern already used elsewhere in the file; pull_request.user.login is set by GitHub's own identity system and isn't attacker-editable, so this isn't a spoofable gate.

Summary

No injection vector, no secret exposure, no auth/access-control impact from this PR. The one item worth tracking is the ${{ github.base_ref }} → shell interpolation style (informational, pre-existing pattern, low real-world exploitability given base_ref's constraints) — recommend a small follow-up hardening PR rather than blocking this one.

[github-actions]

🎉 This PR is included in version 2.13.0-beta.21 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀