fix(workflows): remove literal empty GHA expression from pr-labels-ci.yml comments#92
Conversation
….yml comments Two shell comments inside `run:` blocks of pr-labels-ci.yml carried the literal sequence for an empty GitHub Actions expression. GHA substitutes such sequences inside run: blocks before the shell sees them, including within shell comments, and the queue-time parser rejects the empty form: Failed to queue workflow run: Invalid Argument - failed to parse workflow: An expression was expected The normal workflow_run path tolerated this (evaluated to empty string), so the bug has been latent on mcp-clipboard since #88 landed. It only surfaces on workflow_dispatch or on a fresh-repo cascade where workflow_run registration hasn't happened yet. Root-cause was traced on cmeans/yt-dont-recommend#28 after four separate fix attempts (#24-#27) each produced the same parse error at run: | col 14 (the run: | scope, not the actual offending token). Both comments now describe the concept without the literal characters. Zero logic change; the legitimate non-empty expressions elsewhere in the file are unaffected. Closes #91. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
cmeans
left a comment
There was a problem hiding this comment.
QA Review — PR #92 (Round 1)
Scope: Removes literal empty ${{ }} from two shell comments in .github/workflows/pr-labels-ci.yml. Closes #91. Comment-only change (+2 / −2) plus CHANGELOG.
Verification
- Diff matches issue #91 exactly. Issue called out two sites (around lines 49 and 100); both replaced
(not direct ${{ }} interpolation)with(not a direct GHA expression). Suggested-fix wording in the issue is what Dev used. yaml.safe_load pr-labels-ci.yml→ clean, 2 jobs (on-ci-pass,on-ci-fail).- Residual scan across all 6 workflow files (
rg '\$\{\{\s*\}\}') → zero matches inci.yml,pr-labels-ci.yml,pr-labels.yml,publish.yml,qa-gate.yml,test-publish.yml. - Original rationale is preserved — both comments still describe "HEAD_BRANCH comes via env because fork PR branch names are contributor-controlled."
- Legitimate non-empty
${{ ... }}expressions inenv:blocks untouched:secrets.GITHUB_TOKEN,github.repository,github.event.workflow_run.id,github.event.workflow_run.head_branch. uv run pytest→ 488 passed, 5 xfailed, 6 deselected (integration-marked, run separately).uv run pytest -m integration tests/test_integration.py→ 6 passed.uv run ruff check/ruff format --check/mypy→ all clean.- CHANGELOG under
[Unreleased] / ### Fixed, links #91, accurately describes the latent-until-dispatch-or-fresh-repo nature of the bug.
The bug was genuinely active on this repo
The PR body claims the normal workflow_run path tolerated the empty expression, so the bug was latent. Checking gh run list --workflow .github/workflows/pr-labels-ci.yml:
- 2026-04-20T01:31:49Z —
event=push,head_branch=main, conclusion=failure, 0 jobs run. That push is the merge of #88, which introduced the empty${{ }}into main. - 2026-04-19T22:17:01Z — same pattern on PR #88's branch push, conclusion=failure.
- PR #92's branch push produced no such entry — because the fix removes the literal the parser rejects.
So the GitHub-side "phantom" failure I flagged as "harmless display artifact" on PR #88 was in fact this exact bug (queue-time parser rejecting the empty expression). This PR verifiably eliminates it — a good retroactive confirmation that #91 was worth filing.
CI on head commit
lint / typecheck / test (3.11, 3.12, 3.13) / on-push / on-label / qa-approved / codecov/patch — all green.
Workflow automation on this PR
pr-labels.ymlon-push— success; transitionedAwaiting CI→Ready for QA.qa-gate.yml— success;QA Gateexternal status =pending(expected).pr-labels-ci.ymlon-ci-pass— ran frommainper itsworkflow_runtrigger (now running main's hardened #88 version), success, promoted label.
Findings
None.
Post-merge follow-ups (tracked, not blockers)
- Manual
workflow_dispatchsanity onpr-labels-ci.yml(test plan item 4) is post-merge only. - Unblocks #89 (add
workflow_dispatch:trigger) — must land after this. - Companion cascades on
mcp-awareness#332andmcp-synology#52(plus the #87 hardening) tracked on those repos.
Verdict
Ready for QA Signoff. Awaiting maintainer for QA Approved.
|
Applying Ready for QA Signoff as final act. Zero findings on review. Diff matches issue #91 exactly (2 comment sites fixed with the issue's suggested wording, +2/-2 plus CHANGELOG). Verification: yaml.safe_load clean, residual scan finds zero empty expressions across all 6 workflow files, 488 tests + 6 integration pass, ruff/mypy clean. Also retroactively confirmed the phantom push-event failure I saw on PR #88 was in fact this same parser bug — this PR verifiably eliminates it. Awaiting maintainer QA Approved. |
…on (#332) (#333) Closes #332. ## Summary `.github/workflows/pr-labels-ci.yml` on this repo was in the pre-`cmeans/mcp-clipboard#88` state for the sibling `cmeans/*` cascade: contributor-controlled `workflow_run` fields (`head_branch`, `id`, `repository`) were inlined directly as `${{ ... }}` expressions inside `run:` bodies. Git refnames allow shell metacharacters (`$`, backtick, `;`, `&`, `|`, etc.), so a malicious fork-PR branch name would render as directly-executed shell once GHA substituted the expression at queue time. This PR cascades the hardening from `cmeans/mcp-clipboard`: 1. **#88 pattern — env routing.** All contributor-controlled values now flow through step-level `env:` blocks and are referenced as shell variables (`"$HEAD_BRANCH"`, `"$REPO"`, `"$RUN_ID"`). Job-level `if:` conditionals continue to use `${{ ... }}` (that expression context is safe — it's evaluated by GHA itself, not handed to a shell). 2. **#92 pattern — comment escape.** Comments inside `run:` blocks avoid any literal `${{ }}` sequence. GHA's queue-time parser substitutes expressions even inside shell `#` comments and rejects empty expressions with `An expression was expected` on `workflow_dispatch` / fresh-repo registration. Without this, the #88 hardening would have landed a latent trap. Cascaded verbatim from `cmeans/mcp-clipboard`'s `.github/workflows/pr-labels-ci.yml` at the `main` HEAD after PR #92 merged (2026-04-20T20:26:12Z), preserving this repo's AGPL-3.0 header at lines 1–16. No functional behavior change — label transitions, PR lookup, force-push tolerance, and Dev Active skip logic are all byte-identical to before, just with shell values now arriving via environment instead of via text substitution. ## Scope - `.github/workflows/pr-labels-ci.yml` — hardened (22 lines changed) - `CHANGELOG.md` — `[Unreleased] → ### Security` entry No source, tests, migrations, or docs. ## References - Root diagnosis of the GHA parser quirk: [cmeans/yt-dont-recommend#28](cmeans/yt-dont-recommend#28) - Original security hardening (merged): [cmeans/mcp-clipboard#88](cmeans/mcp-clipboard#88) - Comment-escape cascade source (merged): [cmeans/mcp-clipboard#92](cmeans/mcp-clipboard#92) - Follow-up for symmetric `pr-labels.yml` hardening (non-blocking, P3): [#334](#334) ## QA ### Prerequisites Pure workflow-file change. No deploy, no env setup, no Python changes. All verification is static (`yq`/`python -c`, `grep`) plus an optional post-merge Actions UI smoke. ### Automated checks The CHANGELOG check, ruff, mypy, pytest, Codecov — none of these touch the workflow file. They should all pass unchanged against `main`. Confirm via the green CI checks on this PR before approving. ### Manual tests All local verification runs from the repo root on the PR branch (`fix/pr-labels-ci-hardening-332`). 1. - [x] **YAML parse is clean.** ``` python3 -c "import yaml; yaml.safe_load(open('.github/workflows/pr-labels-ci.yml')); print('OK')" ``` Expected: prints `OK`, exit 0. Confirms no syntax regression. 2. - [x] **No literal empty `${{ }}` anywhere in the file.** ``` grep -n '\${{ }}' .github/workflows/pr-labels-ci.yml || echo "(none — good)" ``` Expected: prints `(none — good)`. Guarantees the `#92` parser trap can't surface on `workflow_dispatch` / fresh-repo registration. 3. - [x] **No contributor-controlled field appears inside a `run:` body as an expression.** ``` awk '/^ run: \|/,/^ - name:|^ [^ ]|^jobs:/' .github/workflows/pr-labels-ci.yml | grep -nE '\$\{\{ *github\.event\.workflow_run\.(head_branch|id) *\}\}' || echo "(none — good)" ``` Expected: prints `(none — good)`. The only remaining `${{ github.event.workflow_run.head_branch }}` references are inside job-level `if:` conditionals (safe context, evaluated by GHA not a shell) and step-level `env:` assignments (safe by design — the whole point of the cascade). 4. - [x] **`env:` blocks route the three contributor-controlled fields.** ``` grep -nE '^[[:space:]]+(REPO|RUN_ID|HEAD_BRANCH):' .github/workflows/pr-labels-ci.yml ``` Expected: six lines total (three per job), each assigning from the matching `${{ github.event.workflow_run.* }}` / `${{ github.repository }}` expression. 5. - [x] **`workflows: [CI]` still matches the CI workflow name in this repo.** ``` grep -A1 'workflows:' .github/workflows/pr-labels-ci.yml | head -3; grep '^name:' .github/workflows/ci.yml ``` Expected: `workflows: [CI]` appears in this file AND `name: CI` appears in `.github/workflows/ci.yml`. Names match, so `workflow_run` will still fire on CI completion. 6. - [x] **AGPL header preserved.** ``` sed -n '1,16p' .github/workflows/pr-labels-ci.yml ``` Expected: lines 1–16 are the existing `mcp-awareness — ambient system awareness for AI agents` / `Copyright (C) 2026 Chris Means` / AGPLv3 preamble. Unchanged from `main`. 7. - [x] **Diff against `main` is hardening-only (no behavior drift).** ``` git diff origin/main -- .github/workflows/pr-labels-ci.yml ``` Expected: only adds `REPO:`, `RUN_ID:`, `HEAD_BRANCH:` to the two `env:` blocks, deletes the shell-level `REPO=${{...}}` / `HEAD_BRANCH=${{...}}` assignments, rewrites `API_OUT=...${{ ... }}/pull_requests` to `.../$RUN_ID/pull_requests`, and updates two comment blocks to avoid `${{ }}` literals. No changes to `if:` conditionals, `permissions`, `on:`, job names, label manipulation logic, or `exit 0` paths. 8. - [ ] **(Post-merge) Smoke the label automation end-to-end.** This workflow is `workflow_run`-triggered (no `workflow_dispatch`), so pre-merge dispatch isn't available. After merge, take any in-flight `Awaiting CI` PR and confirm the label transitions still happen on the next CI completion: - Watch `.github/workflows/pr-labels-ci.yml` runs at https://github.com/cmeans/mcp-awareness/actions/workflows/pr-labels-ci.yml - Expected on CI pass: `on-ci-pass` succeeds (~5 s), target PR moves `Awaiting CI → Ready for QA`. - Expected on CI fail: `on-ci-fail` succeeds, target PR gains `CI Failed`. ### Out of scope (explicitly) - **`pr-labels.yml`** (the `on: pull_request:` sibling) is not in this PR. Its current trigger makes `secrets.GITHUB_TOKEN` read-only for fork PRs and the values it does inline into `run:` blocks today are all safe types (numeric, repo name, hex SHA), so no injection vector exists at the current configuration. Symmetric env-routing as defense-in-depth tracked in [#334](#334) (P3, non-blocking). - **Adding `workflow_dispatch` to `pr-labels-ci.yml`** for post-merge verifiability. Not cascaded because mcp-clipboard doesn't have it either. Track separately if desired. --------- Co-authored-by: cmeans-claude-dev[bot] <3223881+cmeans-claude-dev[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary Closes #52. `.github/workflows/pr-labels-ci.yml` was in its pre-hardening state: both the `on-ci-pass` and `on-ci-fail` jobs inlined `${{ github.event.workflow_run.head_branch }}` directly inside `run:` blocks. `head_branch` is contributor-controlled on fork PRs and git refnames allow shell metacharacters (`$`, backtick, `;`, `&`, `|`, etc.), so a malicious fork branch name would have rendered as directly-executed shell once the expression was substituted. The same hardening has already landed on sibling repos (`cmeans/mcp-clipboard#88`, `cmeans/yt-dont-recommend`), but never cascaded here. ## Change - **`.github/workflows/pr-labels-ci.yml`** — `REPO`, `RUN_ID`, and `HEAD_BRANCH` now come through step-level `env:` blocks on both jobs. The shell references them as `$REPO` / `$RUN_ID` / `$HEAD_BRANCH`. No remaining `${{ ... }}` substitutions inside either `run:` body. - **CHANGELOG entry** added under `## Unreleased` → `### Fixed`. ## Latent-parser-trap note Issue #52 flagged that the verbatim cascade from mcp-clipboard's post-#88 source also carried a shell comment containing a literal empty GHA expression — GHA substitutes `${{ ... }}` inside `run:` blocks before the shell sees them *including within shell comments*, and the queue-time parser rejects an empty expression on `workflow_dispatch`. The explanatory comments in this PR therefore describe the concept ("not a direct GHA expression") rather than showing the literal sequence, matching the fix on `cmeans/mcp-clipboard#92`. This is defense-in-depth today — the trap itself cannot fire on this repo until a `workflow_dispatch:` trigger is added (see *Out of scope* below). ## Test plan - [x] `python3 -c "import yaml; yaml.safe_load(open('.github/workflows/pr-labels-ci.yml'))"` → clean parse. - [x] `grep '${{ }}'` on the file → no matches. - [x] Programmatic scan: no `${{ ... }}` substitution survives inside any `run:` body in the file. - [x] `workflow_run.workflows: [CI]` still matches this repo's CI workflow name (`name: CI` in `ci.yml`). - [ ] After merge: the next CI pass on a PR branch still transitions `Awaiting CI` → `Ready for QA` (and CI fail still sets `CI Failed`). `workflow_run` listeners can only be exercised post-merge on the default branch. ## Out of scope - **Adding a `workflow_dispatch:` trigger** to `pr-labels-ci.yml`. mcp-clipboard defers this to a separate PR (#89 there) and that precedent fits this repo too — keeping issue #52's scope to the hardening cascade. A follow-up issue can pick it up if manual dispatch is wanted; the defensive comment-escape in this PR ensures that follow-up won't trip the queue-time parser. ## Related - Original hardening: `cmeans/mcp-clipboard#88` - Empty-expression parser diagnosis: `cmeans/yt-dont-recommend#28` - Comment-escape cascade: `cmeans/mcp-clipboard#92` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: cmeans-claude-dev[bot] <3223881+cmeans-claude-dev[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…commend#24 (#93) ## Summary Restores template parity for `pr-labels-ci.yml` with `cmeans/yt-dont-recommend`, which diverged by adding a `workflow_dispatch:` trigger in PR #24 (needed over there to prime the `workflow_run` dispatcher after its fresh-repo cascade). The diff applied here is verbatim from that upstream PR. The new trigger has two purposes, documented inline in the file: 1. **Seeding** — in a fresh-repo cascade, the `workflow_run` dispatcher doesn't always register until the workflow has produced at least one non-startup-failure run. A manual dispatch produces a clean skipped run and primes the dispatcher. 2. **Debugging** — gives maintainers a manual "Run workflow" button from the Actions UI without having to piggyback on a real CI run. **No-op on existing PRs.** Both jobs' `if:` guards require `github.event.workflow_run.*`, which are absent on a `workflow_dispatch` event, so a manual run always skips cleanly. **Side benefit:** a manual dispatch now serves as direct confirmation that the queue-time parser accepts this file. Prior to PR #92 landing (removing the literal empty GHA expression from two shell comments), a manual dispatch here would have failed with "An expression was expected" — so this PR is the natural post-fix validation point. Closes #89. ## Test plan - [x] `yaml.safe_load` on the file — clean parse with both triggers present (verified locally; confirms both `workflow_dispatch:` and `workflow_run:` keys present). - [x] CI (unchanged) runs green. - [ ] Post-merge: Actions tab -> "PR Label Automation (CI)" -> "Run workflow" -> main. Expected: clean queue, both jobs skip via `if:` guards, run completes in a few seconds. This is the direct validation that PR #92's fix works at the queue-time parser level. - [ ] Subsequent PRs continue to auto-promote from `Awaiting CI` to `Ready for QA` as before (no regression on the `workflow_run` path). ## Related - Upstream template change: `cmeans/yt-dont-recommend#24` - Prerequisite on this repo: #92 (removed the literal empty GHA expression that would have made a manual dispatch fail) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: cmeans-claude-dev[bot] <3223881+cmeans-claude-dev[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bump pyproject.toml 2.2.1 -> 2.3.0 and convert the [Unreleased] block into [2.3.0] - 2026-05-02. A fresh empty [Unreleased] section sits above for the next cycle. 13 PRs aggregated since v2.2.1: #88, #92, #93, #94, #95, #96, #98, #99, #100, #101, #102, #103, #104. Tag-push (v2.3.0) after merge triggers .github/workflows/publish.yml. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Two shell comments inside
run:blocks of.github/workflows/pr-labels-ci.ymlcarried the literal sequence for an empty GitHub Actions expression. GHA substitutes such sequences insiderun:blocks before the shell sees them — including within shell comments — and the queue-time parser rejects the empty form:The normal
workflow_runpath tolerated it (evaluated to empty string, carried on), so the bug has been latent on mcp-clipboard since #88 landed. It only surfaces onworkflow_dispatch, or on a fresh-repo cascade whereworkflow_runregistration hasn't happened yet.Root cause was traced on
cmeans/yt-dont-recommend#28after four separate fix attempts (#24, #25, #26, #27) each produced the same parse error atrun: |col 14 — the enclosing scope, not the actual offending token.Closes #91.
Change (+2 / −2, comment-only)
Both comments rewritten to describe the concept without the literal characters. Zero logic change; the legitimate non-empty expressions elsewhere in the file (in
env:blocks) are untouched.Test plan
yaml.safe_loadonpr-labels-ci.yml— clean parse (verified locally).grep '${{ }}'on the file returns nothing (verified locally).workflow_runhappy path — CI on this PR exercises the pipeline as usual.workflow_dispatchfrom the Actions UI onpr-labels-ci.ymlqueues cleanly (both jobs skip because theirif:guards requiregithub.event.workflow_run.*, which don't exist onworkflow_dispatch). Optional; nothing in mcp-clipboard today requires manual dispatch to work.workflow_dispatch:trigger) — safe to pick that up only after this lands, otherwise it would trip the exact same parser error.Related
cmeans/yt-dont-recommend#28.workflow_dispatch:trigger from yt-dont-recommend#24). Must land after this.Context
Refiled cleanly from the original PR #90 (closed) after the upstream claude-dev session confirmed the issue-first handoff was the correct process. Issue #91 carries the spec.
🤖 Generated with Claude Code