Skip to content

ci: publish cbioportal/mcp:beta on pushes to beta (main) - #157

Merged
ForisKuang merged 3 commits into
cBioPortal:mainfrom
ForisKuang:polly/beta-image-ci
Sep 30, 2026
Merged

ForisKuang merged 3 commits into
cBioPortal:mainfrom
ForisKuang:polly/beta-image-ci

Conversation

@ForisKuang

@ForisKuang ForisKuang commented Sep 27, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

.github/workflows/docker_ci.yml only. #157 (base main) and #158 (base beta) contain the same change from the same head branch.

Beta tag

  • Added beta to on.push.branches.
  • In both "Set image tag" steps (the build matrix job and the merge job), a push to beta sets IMAGE_TAG=beta.
  • A push to main still sets IMAGE_TAG=latest. workflow_dispatch with a valid ref still tags with that ref.
  • Any other trigger fails the tag step (exit 1) instead of falling back to latest. That includes a workflow_dispatch with an empty ref, which used to publish :latest and now errors. ref is required: true in the dispatch UI, so this only matters for API or CLI dispatches that pass an empty string.

Script-injection hardening (the first sink predates this PR)

  • github.event.inputs.ref, github.event_name and github.ref_name are no longer interpolated with ${{ }} inside run: scripts. They're passed as INPUT_REF / EVENT_NAME / REF_NAME env vars and used as quoted "$VAR". This closes the pre-existing sink, where a dispatcher could inject shell via ref on a runner that holds DOCKER_PASSWORD.
  • A dispatched ref must match ^[A-Za-z0-9_][A-Za-z0-9_.-]{0,127}$ as one whole string, using bash [[ =~ ]], before it's written to $GITHUB_ENV. Both tag steps are pinned to shell: bash and set LC_ALL=C, so the character ranges are ASCII-only.
    • This replaces a grep -Eqx guard from an earlier commit on this branch (4a6d2c1). That guard was bypassable: grep -x anchors each line, so v1<newline>IMAGE_TAG=latest passed and published :latest, and v1<newline>INJECTED=1 wrote INJECTED=1 into $GITHUB_ENV. The whole-string match rejects both (see the table below).
    • Refs with / (e.g. feature/foo) were never valid Docker tags. Before, they failed late at imagetools create; now they fail at the tag step. The input description now says "Docker tag / ref (no slashes)".
  • The merge job's imagetools create and inspect commands use a quoted "$IMAGE_TAG" instead of interpolating ${{ env.IMAGE_TAG }}.
  • Removed the unreferenced id: set-tag.

Build cache

  • The GHA build cache scope is now ${PLATFORM_PAIR}-${IMAGE_TAG}, so beta and main builds don't evict each other's cache. The first build on each tag after merge will be a cold build.

Why

#151–#156 now target beta so they can roll out to beta only. A push to beta has to publish cbioportal/mcp:beta and must never produce or overwrite cbioportal/mcp:latest. A push to beta runs the workflow file on the beta branch, so the change is needed on both main and beta.

Prod impact (read before merging)

  • Merging ci: publish cbioportal/mcp:beta on pushes to beta (main) #157 is a push to main, so it rebuilds and republishes cbioportal/mcp:latest with a new digest. docker/Dockerfile runs COPY . /app and the repo has no .dockerignore, so this workflow file is part of the build context and the image changes. The server code in the image is byte-identical, but Keel will see the new :latest digest and roll the prod MCP pods. Merge when a routine pod restart is acceptable.
  • Merging ci: publish cbioportal/mcp:beta on pushes to beta (beta) #158 is a push to beta. It publishes :beta only and never touches :latest.
  • After both merge, later main pushes publish :latest as before and later beta pushes publish :beta only.

Verification

  • python3 -c "import yaml; yaml.safe_load(open('.github/workflows/docker_ci.yml'))" parses.
  • docker run --rm -v "$PWD":/repo -w /repo rhysd/actionlint:1.7.12 -no-color -oneline .github/workflows/docker_ci.yml (with shellcheck 0.11.0) reports two issues, both in lines this PR doesn't touch:
    • line 30: actions/checkout@v3 is too old
    • line 147: SC2046 on $(printf 'cbioportal/mcp@sha256:%s ' *). The word splitting there is intentional.
    • No findings in either tag step.
  • How the step tests run: both tag-step bodies are pulled from the YAML, which is checked for shell: bash and no ${{ in the body. Each body is written to a file and run the way GHA runs shell: bash, i.e. bash --noprofile --norc -eo pipefail <file>, with INPUT_REF / EVENT_NAME / REF_NAME / LC_ALL=C set and a temporary $GITHUB_ENV.
    • The run passes only if accepted cases write exactly one IMAGE_TAG= line (plus PLATFORM_PAIR in build) and rejected cases exit non-zero and write nothing.
    • Results were identical on bash 5.2.37 (Debian, in python:3.12) and macOS bash 3.2.57: 34/34 pass (17 cases × 2 jobs).
Case Expected build & merge result
push / main latest exit 0, one line IMAGE_TAG=latest
push / beta beta exit 0, one line IMAGE_TAG=beta
dispatch v1 accept exit 0, one line IMAGE_TAG=v1
dispatch v1.2.3 accept exit 0, one line IMAGE_TAG=v1.2.3
dispatch feature-mcp-apps accept exit 0, one line
dispatch main accept exit 0, one line IMAGE_TAG=main
dispatch 40-char SHA accept exit 0, one line
dispatch 128-char tag accept exit 0, one line
dispatch empty ref reject exit 1, $GITHUB_ENV empty
dispatch v1\nINJECTED=1 reject exit 1, $GITHUB_ENV empty
dispatch v1\nIMAGE_TAG=latest reject exit 1, $GITHUB_ENV empty
dispatch v1\n (trailing newline) reject exit 1, $GITHUB_ENV empty
dispatch feature/foo reject exit 1, $GITHUB_ENV empty
dispatch $(id) reject exit 1, $GITHUB_ENV empty
dispatch `id` reject exit 1, $GITHUB_ENV empty
dispatch x"; echo PWNED; " reject exit 1, $GITHUB_ENV empty, no PWNED output
dispatch 129-char tag reject exit 1, $GITHUB_ENV empty

Regression check: the same harness run against the previous grep -Eqx guard (4a6d2c1) fails 6 checks: the three newline cases × 2 jobs. For example, v1\nIMAGE_TAG=latest exits 0 and writes ['IMAGE_TAG=v1', 'IMAGE_TAG=latest'], which confirms the bypass and shows the harness catches it.

Open follow-ups (not in this PR)

  • Reject latest as a dispatch ref. A dispatcher can still pass ref=latest and overwrite :latest from any branch. That was already true before this PR and is left as-is here.
  • Add a concurrency group. Two quick pushes to the same branch can race in the merge job, and the older run could tag last.

🤖 Generated with Claude Code

Foris Kuang and others added 2 commits September 26, 2026 20:00
Add `beta` to the push trigger and tag images built from a beta push as
`beta` in both the build and merge jobs, so a beta push never produces or
overwrites `latest`. main pushes still publish `latest` and
workflow_dispatch still tags with the provided ref. Any other trigger now
fails the tag step instead of silently defaulting to `latest`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
Stop interpolating github.event.inputs.ref / event_name / ref_name into
the tag-step run scripts; pass them as INPUT_REF / EVENT_NAME / REF_NAME
env vars instead. Validate a dispatched ref as a Docker tag before writing
it to $GITHUB_ENV (rejects newlines and shell metacharacters), and quote
$IMAGE_TAG in the imagetools create/inspect commands instead of
interpolating env.IMAGE_TAG. Scope the GHA build cache per image tag so
beta and main builds don't evict each other.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
The grep -Eqx guard anchored per line, so a ref like
"v1<newline>IMAGE_TAG=latest" passed (first line is tag-shaped) and the
extra line was written into $GITHUB_ENV, publishing :latest or injecting
arbitrary env vars. Replace it in both tag steps with a bash [[ =~ ]]
whole-string match, pin those steps to `shell: bash`, and set LC_ALL=C
so the [A-Za-z] ranges are ASCII-only.

Also clarify the dispatch input description (Docker tag / ref, no
slashes) and drop the unreferenced `id: set-tag`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@ForisKuang

Copy link
Copy Markdown
Collaborator Author

@inodb how have you been publishing to beta chat? wanted to make sure that I'm not doing something redundant.

@ForisKuang
ForisKuang merged commit bd27034 into cBioPortal:main Sep 30, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant