resolve_release_ref falls back to a branch name inside a release-tag URL, so an unreachable API becomes an anonymous 404 #1346

Open
opened 2026-08-28 18:15:31 +00:00 by coilyco-ops · 0 comments
Owner

Split out of #1345 rather than folded into it, because it is a behavior change to a resolver four products share and deserves its own review. Caught by Delphi reviewing that cut.

The defect

agentic_os/prod_install_ref.resolve_release_ref reads the release branch head, finds the version tag at that sha, and on any KeyError/TypeError/OSError/ValueError returns the literal string "release", described in its own docstring as "a safe fallback".

It is not safe. Every caller interpolates the result into a release-tag download path:

base=".../releases/download/${ref}"

A branch name in a tag path resolves to nothing. There is no release tag, and the tag schemes are v* and aos-v*, so there never should be. The fallback therefore names a URL that cannot succeed, for any product, under any conditions.

What it cost

During the #1344 outage this converted "the Forgejo API was unreachable" into six identical anonymous lines:

curl: (22) The requested URL returned error: 404

No URL, no ref, no underlying exception. Two of us independently spent an hour reading it as a URL or packaging bug before the real cause (the runner losing reach to forgejo.coilysiren.me) turned up in a different job's log. The exception that would have named it was swallowed by the except clause.

This sits directly against the fail-closed discipline the very next lines of install-workflow-ward.sh were written for, where a missing checksum line is deliberately made to exit non-zero rather than pass quietly.

Scope after #1345

#1345 removes the ward product from PRODUCTS, which does not fix this. aos, umbra, specgen, and guard all still resolve through the same function and the same fallback, and the dev-base image builds install exactly this way.

Two shapes, either acceptable

  1. Fail closed. Raise with the underlying exception rather than returning a ref that cannot resolve. Callers die with a readable cause instead of an anonymous 404. Changes behavior on a transient API blip from "confusing failure" to "clear failure", never from success to failure, since the fallback path could not succeed anyway.
  2. Validate at the boundary. Keep the fallback and have each installer refuse a ref that does not match the product's tag pattern, naming what it got.

(1) is preferred: it puts the guard in the one place that knows what went wrong. Whichever lands, the installers should echo the resolved ref so the next failure of this shape names itself.

Verifying a fix

The negative control matters more than the positive one here, since the bug is a silent pass. A test must assert that an unreachable API produces an error naming the cause, not a string. tests/test_prod_install_ref.py::test_literal_release_is_the_fallback_when_forgejo_is_unavailable currently pins the broken behavior and has to be inverted, not deleted.

Split out of #1345 rather than folded into it, because it is a behavior change to a resolver four products share and deserves its own review. Caught by Delphi reviewing that cut. ## The defect `agentic_os/prod_install_ref.resolve_release_ref` reads the `release` branch head, finds the version tag at that sha, and on **any** `KeyError/TypeError/OSError/ValueError` returns the literal string `"release"`, described in its own docstring as "a safe fallback". It is not safe. Every caller interpolates the result into a **release-tag download path**: ``` base=".../releases/download/${ref}" ``` A branch name in a tag path resolves to nothing. There is no `release` tag, and the tag schemes are `v*` and `aos-v*`, so there never should be. The fallback therefore names a URL that **cannot** succeed, for any product, under any conditions. ## What it cost During the #1344 outage this converted "the Forgejo API was unreachable" into six identical anonymous lines: ``` curl: (22) The requested URL returned error: 404 ``` No URL, no ref, no underlying exception. Two of us independently spent an hour reading it as a URL or packaging bug before the real cause (the runner losing reach to `forgejo.coilysiren.me`) turned up in a *different* job's log. The exception that would have named it was swallowed by the `except` clause. This sits directly against the fail-closed discipline the very next lines of `install-workflow-ward.sh` were written for, where a missing checksum line is deliberately made to exit non-zero rather than pass quietly. ## Scope after #1345 #1345 removes the `ward` product from `PRODUCTS`, which does **not** fix this. `aos`, `umbra`, `specgen`, and `guard` all still resolve through the same function and the same fallback, and the dev-base image builds install exactly this way. ## Two shapes, either acceptable 1. **Fail closed.** Raise with the underlying exception rather than returning a ref that cannot resolve. Callers die with a readable cause instead of an anonymous 404. Changes behavior on a transient API blip from "confusing failure" to "clear failure", never from success to failure, since the fallback path could not succeed anyway. 2. **Validate at the boundary.** Keep the fallback and have each installer refuse a ref that does not match the product's tag pattern, naming what it got. (1) is preferred: it puts the guard in the one place that knows what went wrong. Whichever lands, the installers should echo the resolved ref so the next failure of this shape names itself. ## Verifying a fix The negative control matters more than the positive one here, since the bug **is** a silent pass. A test must assert that an unreachable API produces an error naming the cause, not a string. `tests/test_prod_install_ref.py::test_literal_release_is_the_fallback_when_forgejo_is_unavailable` currently pins the broken behavior and has to be inverted, not deleted.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-flight-deck/agentic-os#1346
No description provided.