ci: add manual workflow to validate DOCKERHUB_TOKEN before a build#716
Merged
kojiromike merged 7 commits intoMay 12, 2026
Merged
Conversation
kojiromike
added
the
github_actions
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a manually triggered GitHub Actions workflow intended to quickly validate DOCKERHUB_USERNAME / DOCKERHUB_TOKEN against both Docker Hub registry login and the Docker Hub API path used by the README/description updater, to support future secret rotations (refs #714).
Changes:
- Introduces a new
workflow_dispatchworkflow to check Docker Hub registry authentication viadocker/login-action. - Adds a scripted Docker Hub API login + repository probe to surface clearer diagnostics for invalid vs. unauthorized credentials.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Issue openemr#714 had to recover from a Forbidden response on the readme push after the DOCKERHUB_TOKEN secret silently lost the right scope. The only ways to confirm a rotation worked were edit OVERVIEW.md and wait for the dockerhub-description workflow to fire, or trigger a full nightly build — both expensive ways to discover the token is still wrong. Add a workflow_dispatch that exercises both auth paths the build workflows depend on: - docker/login-action — registry-level auth (the path image-push uses) - Direct Docker Hub API auth via /v2/users/login/ + /v2/repositories/<repo>/ — the path peter-evans/dockerhub-description uses for PATCH A token can pass the first and fail the second, which is exactly the failure mode openemr#714 had to recover from. Logic lives under the existing release tooling pattern at tools/release/: - src/DockerHubCredentialChecker.php — orchestrates the two HTTP calls - src/DockerHubCredentialCheckResult.php — pure result interpretation (the testable surface, no HTTP mocking required in tests) - src/DockerHubCredentialCheckStatus.php — outcome enum - bin/check-dockerhub-credential.php — Symfony Console one-shot - tests/DockerHubCredentialCheckResultTest.php — covers every status - Taskfile entry: ci:check-dockerhub-credential The workflow becomes checkout → docker login → setup-php + setup-task → `task ci:check-dockerhub-credential`. ext-curl is added to composer requires so composer-require-checker stays clean. Refs openemr#714. Assisted-by: Claude Code
kojiromike
force-pushed
the
dockerhub-credential-check
branch
from
17762e6 to
b912a19
Compare
Per review feedback: GET /v2/repositories/<repo>/ only confirms read access. A read-only token would still pass that check but 403 on the actual PATCH the readme push uses, producing a false positive. Read the current description / full_description, then PATCH the same values back. That exercises the exact endpoint peter-evans/dockerhub-description uses, so the smoke test now fails for a token that lacks write scope. Side effect: the no-op PATCH bumps the repo's last-modified timestamp on Docker Hub — the real readme push already does this every run, so nothing novel. Update interpret() to take separate read and write statuses; add a test covering the read-OK / write-403 path. Assisted-by: Claude Code
… errors Per review feedback: - Non-200 GET / non-200 PATCH responses were always classified as INSUFFICIENT_SCOPE, which would mislabel a 500/404/rate-limit as a permissions issue and prompt unnecessary secret rotation. Map only 401/403 to permission failures (INVALID_CREDENTIAL on login, INSUFFICIENT_SCOPE on access). Everything else routes to UNEXPECTED_RESPONSE with the actual HTTP status surfaced. - json_decode + JSON_THROW_ON_ERROR could escape the checker on a non-JSON response (HTML error page from a CDN/WAF, partial body, etc.) and abort the workflow without the intended `::error::` line. Catch JsonException internally and signal "unparseable response" through the existing return tuple. Wrap each step in check() in a try/catch for transport-level RuntimeException and route through a new NETWORK_ERROR status so the workflow always emits exactly one diagnostic line. interpret() now takes the full set of step inputs (login status, jwt, read status, descriptions-parsed flag, write status) and routes via small private factories. Tests cover every status branch (93 tests total, 16 specific to this class). Assisted-by: Claude Code
Per review feedback: substituting empty strings for missing `description` / `full_description` and PATCHing them back would clear the live Docker Hub repo description if the API shape ever changed (field renamed, becomes nullable, etc.). Strict-check both fields. If either is missing or non-string, fetchDescriptions() returns null, which interpret() routes to UNEXPECTED_RESPONSE without issuing the PATCH. Better to surface a diagnostic than silently overwrite real content with empty strings. Already-existing test (testUnexpectedResponseWhenRead200ButUnparseable) covers the resulting interpret() path end-to-end. Assisted-by: Claude Code
Per review feedback: toGithubActionsLine() interpolated repository and detail directly into the `::error::` / `::notice::` workflow command. GitHub Actions parses workflow commands per-line, so an embedded newline could inject a second command (e.g. set-output, set-env). Two-layer defense: - bin/check-dockerhub-credential.php now validates --repository against the standard Docker Hub owner/name pattern up front, rejecting anything else with a clear error. - DockerHubCredentialCheckResult constructor scrubs CR/LF from repository and detail before storing them. Belt-and-braces, so any future caller that bypasses bin still gets a safe single-line render. Tests cover that repository and detail with embedded \r\n produce single-line output with no carriage returns or newlines. Assisted-by: Claude Code
Per review feedback: matches the pinning convention used by build-release.yml and build-patch.yml. Taskfile declares `version: '3'`, so an upstream default change to setup-task wouldn't unexpectedly land us on a Task 4.x runtime that re-interprets the schema. Assisted-by: Claude Code
Per review feedback: at the read and write probes the JWT is already proven valid by the prior login. A 401 on those endpoints means the token isn't accepted by this resource (rotate the credential); a 403 means it's accepted but lacks scope (grant scope). Two different remediations — they should produce two different statuses in the operator-facing output line. fromAccessFailure() and fromWriteStatus() now route 401 → INVALID_CREDENTIAL and 403 → INSUFFICIENT_SCOPE distinctly. New tests lock both 401 paths in. Login (fromAuthFailure) keeps 401 and 403 both as INVALID_CREDENTIAL — at the login step neither distinction is meaningful (the call is "are you who you say you are", and a 403 there is "no, in any flavor"). Assisted-by: Claude Code
This was referenced May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #714
Short description of what this resolves:
Adds a
workflow_dispatchworkflow that provesDOCKERHUB_USERNAME/DOCKERHUB_TOKENare valid for the readme-push API path, without triggering an image build or modifying the readme content. Whoever rotates the secret in the future (per #714) can click "Run workflow" and know in seconds whether the new credential works.Why two checks
A Docker Hub token can pass
docker login(registry auth) and still fail onPATCH /v2/repositories/openemr/openemr/(API auth), because the API path needs Read+Write+Delete scope on the specific repo while the registry only requires push permission. That is exactly the failure mode #714 had to recover from — theUpdate Docker Hub Descriptionworkflow was 403'ing while image pushes kept working.Side effect to be aware of
The API check writes the current description back to itself via a no-op
PATCHso it actually exercises the write scope (a read-only token would 200 on aGETand 403 on thePATCH). The readme content does not change, but Docker Hub'slast_modifiedtimestamp on the repo is bumped. The real readme push already does this on every run, so nothing novel.Changes proposed in this pull request:
.github/workflows/dockerhub-credential-check.yml—workflow_dispatchonly. Steps: checkout →docker/login-action(validates registry auth) → setup PHP + Task →task ci:check-dockerhub-credential(validates the API-path auth via login + GET + no-op PATCH).tools/release/matching the existing pattern (cf.bin/render-dockerhub-overview.phpin feat(ci): render Docker Hub readme from versions.yml at build time #715):src/DockerHubCredentialChecker.php— orchestratesPOST /v2/users/login/,GET /v2/repositories/<repo>/, then no-opPATCH /v2/repositories/<repo>/viaext-curl. CatchesJsonExceptioninternally andRuntimeExceptionfrom the HTTP layer so the workflow always emits exactly one diagnostic line.src/DockerHubCredentialCheckResult.php— pure result interpretation; the testable surface, no HTTP mocking required. Maps 401/403 to credential/scope failures and other non-200s toUNEXPECTED_RESPONSEwith the actual HTTP status surfaced.src/DockerHubCredentialCheckStatus.php—OK/INVALID_CREDENTIAL/INSUFFICIENT_SCOPE/UNEXPECTED_RESPONSE/NETWORK_ERRORenum.bin/check-dockerhub-credential.php— Symfony Console one-shot.tests/DockerHubCredentialCheckResultTest.php— covers every status / step-failure path (16 cases).ci:check-dockerhub-credential.ext-curladded tocomposer.jsonrequires (declared explicitly socomposer-require-checkerstays clean).Test plan
composer checkclean (phpcs, phpstan, rector dry-run, require-checker, 93 phpunit tests).actionlintclean.::error::line distinguishing "invalid credential" (HTTP 401/403 from/users/login/) from "lacks scope" (login OK but 403 from the repo endpoint). After fix(ci): rotate revoked DOCKERHUB_TOKEN — readme workflow returning Forbidden #714 lands and the secret is rotated, re-trigger and expect a::notice::"Credential is valid" line and a green check.