Skip to content

Pre-existing defects found reviewing #33: wrong APT default in README, digest-ref parsing, unanchored sed, hardcoded org, no AGENTS.md #34

Description

@mairas

Five pre-existing defects surfaced by the review of #33 (now closed). Grouped because they are all small and all in the same two files; split them out if any turns out to be bigger than it looks. Each one below was verified directly, not inferred from the code.

1. The README documents the wrong APT repository default

README.md lines 89 and 136 both give:

| `apt-repository` | `hatlabs/apt.hatlabs.fi` | APT repo to dispatch to |

The workflows default to the other one — build-release.yml:51 and publish-stable.yml:23 are both default: 'halos-org/apt.halos.fi'.

This is the most likely of the five to cause real damage. A hatlabs repo author reading the README concludes no override is needed and ships packages to the wrong APT repo. The workspace AGENTS.md states the opposite of the README: "Repos in hatlabs must explicitly set apt-repository: hatlabs/apt.hatlabs.fi (the default is halos-org/apt.halos.fi)."

2. Digest-pinned images are parsed into nonsense and warn daily

check-image-updates.sh splits on the last colon:

>>> 'busybox:1.37@sha256:9532d8c'.rsplit(':', 1)
['busybox:1.37@sha256', '9532d8c']

halos-core-containers/docker-compose.yml:196 pins exactly that shape. The ref goes to the Docker Hub API, which cannot resolve it, and every daily run prints WARNING: Failed to fetch tags, skipping. Nothing is miswritten — it fails closed — but that image has never actually been checked, and the log blames the network for a parse bug. A standing daily warning is also somewhere a real failure can hide.

Fix: skip refs containing @ before splitting, and say the pin is a digest rather than implying a fetch failure.

3. The tag rewrite is an unanchored substring replace using an unescaped regex

sed -i "s|${image}:${current_tag}|${image}:${latest}|g" "$file"

Two problems. The match is unanchored, so an image whose ref is a suffix of another image's ref in the same file gets rewritten too — a compose file pinning both homarr:1.2.3 and ghcr.io/hatlabs/homarr:1.2.3 has both lines rewritten when only one was checked. And ${image} is interpolated into a regex without escaping, so the dots in ghcr.io are wildcards.

Neither fires today: no consumer pairs an upstream image with our fork of it in one file. It becomes live the first time one does, which is a relationship homarr, opencpn and signalk-server each already have with their upstreams.

4. The reusable workflow hardcodes the org for its own script checkout

check-image-updates.yml resolves the ref dynamically from github.workflow_ref (lines 42-48), then hardcodes the repository on line 53:

repository: halos-org/shared-workflows

The workspace AGENTS.md says each org keeps its own copy and that hatlabs repos should reference hatlabs/shared-workflows. A hatlabs caller following that rule still fetches the scripts from halos-org. It works silently while both pin @main and diverges the moment either is pinned to a ref.

The repository should be derived from github.workflow_ref alongside the ref that already is.

5. This repo has no AGENTS.md

Every other repo in the workspace has one, and the workspace AGENTS.md points readers at them for per-repo detail including the changelog scheme. Its absence meant the review had to derive the version-bump answer from the filesystem — no VERSION, no debian/, no bumpversion config, every workflow workflow_call-only — on the repo that carries version-bump-check for everyone else.

Worth stating explicitly in it: this repo ships no .deb and has no VERSION, so the workspace version-bump procedure does not apply; consumers pin @main, so a change to main reaches every consumer's next cron with no staged rollout; and there is a second copy under the hatlabs org that changes need mirroring to.

Related

The hatlabs/shared-workflows copy is behind — its scripts/check-image-updates.sh is still at blob df8e619, and its main has not moved since March. Nothing calls it today, so nothing is broken, but the drift is invisible from here.

Also worth noting for whoever picks this up: the repo has no test harness and no CI on its own PRs, since every workflow is workflow_call-only. The available verification is the dry-run input on check-image-updates.yml, pointed at a consumer branch. That is cheaper than standing up a harness and would have caught the error that sank #33.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions