amtronics/platform-actions

Watch

Fork 0

fix: Treat a rejected Docker Hub token like a missing one #51

Merged

ahmad merged 1 commit from feature/50-tolerate-rejected-login into main

2026-09-05 18:32:36 +00:00

ahmad commented

2026-09-05 18:04:59 +00:00

Owner

Issue

Closes #50

Problem

The Log in to Docker Hub step in templates/release.yml exists on one principle, stated in its own comment: a missing token must not stop a release. That is why it is hand-rolled rather than docker/login-action, which fails a job on an empty password.

It only honoured that for an absent token. A present but rejected one — expired, revoked, rotated — failed the step under set -euo pipefail, taking publish with it. An expired token is the likelier future state than a missing one: tokens are registered once and then quietly age out, and nobody finds out until a release breaks. The failure is avoidable, because the anonymous path that would have worked is right there.

Solution

Treat a failed login exactly like a missing one — warn, and continue anonymously.

Review notes

Driven with a stub docker on an otherwise empty PATH, counting invocations. Before and after, so the change is visible rather than asserted:

Case	before	after
neither secret set	exit 0, warns, `docker` not invoked	unchanged
only `DOCKER_HUB_USER` set	exit 0, warns, `docker` not invoked	unchanged
both set, login accepted	exit 0, "Authenticated as …"	unchanged
both set, login rejected	exit 1 — step and release fail	exit 0, warns, build continues

The first three are regression guards: they pass on both heads and prove nothing about the fix, they only pin what must not change. The fourth is the kill.

check-actions.py reports templates/release.yml: ok (7 run step(s) checked), its own suite passes 31/31, and README lint is clean.

Risks and trade-offs

A build now proceeds anonymously where it previously failed loudly, so a stale credential is discoverable only from the warning rather than from a red job. That is the intended trade and the same one the missing-token branch already makes — an anonymous pull works until the per-IP cap is reached, and a release blocked by a rotated token is a worse outcome than one that pulls anonymously.

Found in review of ahmad/portfolio#76. That PR copies this step verbatim, so it and the remaining ten repositories of #42 should pick this up rather than each fixing it locally — which is exactly the drift #38 exists to end.

### Issue Closes #50 ### Problem The `Log in to Docker Hub` step in `templates/release.yml` exists on one principle, stated in its own comment: a missing token must not stop a release. That is why it is hand-rolled rather than `docker/login-action`, which fails a job on an empty password. It only honoured that for an **absent** token. A **present but rejected** one — expired, revoked, rotated — failed the step under `set -euo pipefail`, taking `publish` with it. An expired token is the likelier future state than a missing one: tokens are registered once and then quietly age out, and nobody finds out until a release breaks. The failure is avoidable, because the anonymous path that would have worked is right there. ### Solution Treat a failed login exactly like a missing one — warn, and continue anonymously. ### Review notes Driven with a stub `docker` on an otherwise empty `PATH`, counting invocations. Before and after, so the change is visible rather than asserted: | Case | before | after | | --- | --- | --- | | neither secret set | exit 0, warns, `docker` not invoked | unchanged | | only `DOCKER_HUB_USER` set | exit 0, warns, `docker` not invoked | unchanged | | both set, login accepted | exit 0, "Authenticated as …" | unchanged | | **both set, login rejected** | **exit 1 — step and release fail** | **exit 0, warns, build continues** | The first three are regression guards: they pass on both heads and prove nothing about the fix, they only pin what must not change. The fourth is the kill. `check-actions.py` reports `templates/release.yml: ok (7 run step(s) checked)`, its own suite passes 31/31, and README lint is clean. ### Risks and trade-offs A build now proceeds anonymously where it previously failed loudly, so a stale credential is discoverable only from the warning rather than from a red job. That is the intended trade and the same one the missing-token branch already makes — an anonymous pull works until the per-IP cap is reached, and a release blocked by a rotated token is a worse outcome than one that pulls anonymously. Found in review of `ahmad/portfolio#76`. That PR copies this step verbatim, so it and the remaining ten repositories of #42 should pick this up rather than each fixing it locally — which is exactly the drift #38 exists to end.

ahmad added 1 commit

2026-09-05 18:05:01 +00:00

fix: Treat a rejected Docker Hub token like a missing one

PR / validate (pull_request) Successful in 4m25s

Details

6dcc63eb51

The step exists so a missing token cannot stop a release, but a present
and rejected one still did: under pipefail the failed login ended the step
and the publish job with it.

An expired or revoked credential is likelier than an absent one, and the
anonymous fallback works in both cases.

ahmad changed title from ~~WIP: fix: Treat a rejected Docker Hub token like a missing one~~ to fix: Treat a rejected Docker Hub token like a missing one

2026-09-05 18:10:25 +00:00

ahmad commented

2026-09-05 18:10:39 +00:00

Author

Owner

Marked ready on 6dcc63e. validate green.

Verified against each acceptance criterion of #50, by extracting the step's run: body and driving it with a stub docker on an otherwise empty PATH, counting invocations so "did not run" is distinguishable from "ran and said nothing":

Both secrets set, login rejected → warns, exits 0. exit=0, one docker call, ::warning::Docker Hub login failed; pulling base images anonymously…. Against the unfixed template the identical input gives exit=1 — that is the kill, and the only case that changes.
Both secrets set, login accepted → reports success, build proceeds authenticated. exit=0, one call, Authenticated to Docker Hub as someone.
Either secret absent → warns, docker never invoked. Both sub-cases exit=0 with zero docker calls.
The rejection warning states pulls will be anonymous and subject to the per-IP limit. It does, in the same wording as the missing-token branch.

Criteria 2 and 3 pass on both heads. They are regression guards pinning what must not change, not evidence the fix works — only criterion 1 is a kill, and a table that blurred the two would read as four proofs when it contains one.

Repository gates: check-actions.py reports templates/release.yml: ok (7 run step(s) checked), its own suite is 31/31, README lint clean.

Note on what does not change: the token still reaches docker through --password-stdin from a printf pipe, so it never enters argv. The pipe is now an if condition, which alters nothing about how the secret is passed — worth stating explicitly since the review of ahmad/portfolio#76 confirmed that property on the original form.

Marked ready on `6dcc63e`. `validate` green. **Verified against each acceptance criterion of #50**, by extracting the step's `run:` body and driving it with a stub `docker` on an otherwise empty `PATH`, counting invocations so "did not run" is distinguishable from "ran and said nothing": 1. *Both secrets set, login rejected → warns, exits 0.* `exit=0`, one `docker` call, `::warning::Docker Hub login failed; pulling base images anonymously…`. **Against the unfixed template the identical input gives `exit=1`** — that is the kill, and the only case that changes. 2. *Both secrets set, login accepted → reports success, build proceeds authenticated.* `exit=0`, one call, `Authenticated to Docker Hub as someone.` 3. *Either secret absent → warns, `docker` never invoked.* Both sub-cases `exit=0` with **zero** `docker` calls. 4. *The rejection warning states pulls will be anonymous and subject to the per-IP limit.* It does, in the same wording as the missing-token branch. Criteria 2 and 3 pass on both heads. They are regression guards pinning what must not change, not evidence the fix works — only criterion 1 is a kill, and a table that blurred the two would read as four proofs when it contains one. Repository gates: `check-actions.py` reports `templates/release.yml: ok (7 run step(s) checked)`, its own suite is 31/31, README lint clean. **Note on what does not change:** the token still reaches `docker` through `--password-stdin` from a `printf` pipe, so it never enters `argv`. The pipe is now an `if` condition, which alters nothing about how the secret is passed — worth stating explicitly since the review of `ahmad/portfolio#76` confirmed that property on the original form.

ahmad_bot approved these changes

2026-09-05 18:31:27 +00:00

ahmad_bot left a comment

Approved. CI green, and I reproduced the whole table against a stub docker on an otherwise empty PATH, counting invocations so "did not call" is measured rather than inferred:

                        before                     after
neither set             exit 0, 0 calls            unchanged
only DOCKER_HUB_USER    exit 0, 0 calls            unchanged
both set, accepted      exit 0, 1 call             unchanged
both set, REJECTED      exit 1, 1 call             exit 0, 1 call, warns

The fourth row is the fix and the other three are unchanged, exactly as described.

Naming the first three as regression guards rather than counting them as kills is the right call, and it is the same distinction that came up on #49 two changes ago — applied here without being asked. A four-row table where three rows pass on both heads reads as much stronger evidence than it is unless someone says so.

One thing the fix buys that the description does not claim. Wrapping the pipeline in if also makes a missing docker binary degrade gracefully, because a command not found (127) lands in the same else:

docker not installed at all    after: exit 0, 0 calls, warns

Before, that would have failed the release too. Not a case that arises on this runner, but it is a real widening of the tolerance and it comes free with the shape you chose over, say, || true on the login alone.

set -e does not interfere, which is the thing most likely to be wrong in this rewrite: a command in an if condition is exempt, so the failing pipeline selects the else branch instead of ending the step. The measured exit 0 confirms it rather than my reading of the manual.

Routing this upstream first was the correct order. ahmad/portfolio#76 carries a byte-identical copy — I verified that when reviewing it — so fixing it there alone would have started the drift #38 exists to end, and the remaining repositories of #42 now inherit the corrected version instead.

Approved. CI green, and I reproduced the whole table against a stub `docker` on an otherwise empty `PATH`, counting invocations so "did not call" is measured rather than inferred: ``` before after neither set exit 0, 0 calls unchanged only DOCKER_HUB_USER exit 0, 0 calls unchanged both set, accepted exit 0, 1 call unchanged both set, REJECTED exit 1, 1 call exit 0, 1 call, warns ``` The fourth row is the fix and the other three are unchanged, exactly as described. **Naming the first three as regression guards rather than counting them as kills is the right call**, and it is the same distinction that came up on `#49` two changes ago — applied here without being asked. A four-row table where three rows pass on both heads reads as much stronger evidence than it is unless someone says so. **One thing the fix buys that the description does not claim.** Wrapping the pipeline in `if` also makes a *missing* `docker` binary degrade gracefully, because a `command not found` (127) lands in the same `else`: ``` docker not installed at all after: exit 0, 0 calls, warns ``` Before, that would have failed the release too. Not a case that arises on this runner, but it is a real widening of the tolerance and it comes free with the shape you chose over, say, `|| true` on the login alone. **`set -e` does not interfere**, which is the thing most likely to be wrong in this rewrite: a command in an `if` condition is exempt, so the failing pipeline selects the `else` branch instead of ending the step. The measured `exit 0` confirms it rather than my reading of the manual. **Routing this upstream first was the correct order.** `ahmad/portfolio#76` carries a byte-identical copy — I verified that when reviewing it — so fixing it there alone would have started the drift `#38` exists to end, and the remaining repositories of `#42` now inherit the corrected version instead.

ahmad merged commit eebd827f9b into main

2026-09-05 18:32:36 +00:00

ahmad referenced this pull request from a commit

2026-09-05 18:32:38 +00:00

fix: Treat a rejected Docker Hub token like a missing one (#51)

ahmad deleted branch feature/50-tolerate-rejected-login

2026-09-05 18:32:38 +00:00