Automate hsm-collector vcpkg registry version publish

[lotgon]

Follow-up to #1267 (the vcpkg registry). Replaces the manual "publish a new version" recipe with a workflow, so bumping the registry to a new collector version is one tag push, not a checklist to remember.

.github/workflows/hsm-collector-registry-publish.yml — on a collector-v<semver> tag push (or manual dispatch):

1. computes the source tarball SHA512,
2. updates ports/hsm-collector/ (portfile REF + SHA512, vcpkg.json version),
3. refreshes versions/ (baseline + version→git-tree),
4. opens a PR — validated by the hsm-collector-registry lane before a human merges.

Manual dispatch defaults to dry_run (shows the computed diff, no PR). Inert until triggered.

Note: the auto-opened PR is created with GITHUB_TOKEN, so the registry check may need a manual re-trigger on it (GitHub suppresses workflow-on-workflow triggers); the PR body says so. A repo PAT would remove that step if desired.

🤖 Generated with Claude Code

[github-actions]

PR #1268 review — hsm-collector-registry-publish workflow

Nice piece of automation — the "commit the port first, capture the git-tree, then amend versions/" dance is exactly the subtlety that trips people up when hand-editing a vcpkg registry, and it's handled correctly here. The dry_run default of true for manual runs is a good safe default. A few issues below, ordered by severity.

🔴 Bugs / correctness

1. The SHA512 length guard is defeated by a missing pipefail — a nonexistent tag silently publishes an empty-tarball hash.

- name: Compute source tarball SHA512
  id: sha
  run: |
    url="https://github.com/${{ github.repository }}/archive/${{ steps.v.outputs.tag }}.tar.gz"
    sha=$(curl -fsSL "$url" | sha512sum | awk '{print $1}')
    [ "${#sha}" -eq 128 ] || { echo "unexpected sha512: $sha"; exit 1; }

This step has no set -o pipefail. In curl | sha512sum | awk, the pipeline's exit status is awk's, so a failed curl -f (e.g. the tag doesn't exist → HTTP 404, empty output) is ignored. sha512sum then hashes an empty stream, producing cf83e1357eefb8bdf... — which is exactly 128 hex chars, so the length check passes and the workflow happily records the SHA512 of an empty tarball. Consumers would then get a hard-to-diagnose hash-mismatch failure at install time.
Fix: add set -euo pipefail to this step (as the "Prepare registry update" step already does), and/or validate curl independently (download to a file, check the exit code, then hash the file).

2. PRs created with GITHUB_TOKEN do not trigger the hsm-collector-registry check the body tells the reviewer to rely on.
The PR body says "ensure the hsm-collector-registry check passes, then merge." By GitHub's design, events (push/pull_request) generated using the default GITHUB_TOKEN do not trigger other workflow runs. So unless that validation lane runs on something else (e.g. merge_group, or is manually re-run), the check will simply be absent and a human may merge with nothing having validated the port. Either use a PAT / GitHub App token for the push+gh pr create, or reword the body to explicitly instruct re-running the check manually. Worth confirming which lane hsm-collector-registry triggers on.

🟠 Security

3. Shell injection via the workflow_dispatch version input (interpolated into the shell script).

tag="collector-v${{ github.event.inputs.version }}"

GitHub expands ${{ ... }} into the script text before the shell parses it, so an input like $(...) or "; <cmd>; " executes arbitrary commands in the runner. It also flows into >> "$GITHUB_OUTPUT", enabling step-output injection (e.g. a newline in the input can forge additional outputs). Note the push branch already does this correctly with tag="$GITHUB_REF_NAME".
Fix: pass the input through env: and reference the shell variable, never the template directly:

env:
  VERSION: ${{ github.event.inputs.version }}
run: |
  tag="collector-v${VERSION}"

Access to workflow_dispatch requires write permission, so the blast radius is limited, but this is a well-known anti-pattern and cheap to fix. The same value later reaches steps.v.outputs.* and is embedded in single-quoted assignments in "Prepare registry update" (ver='${{ steps.v.outputs.ver }}'); a ' in the value would break out of those quotes too — env-passing removes that class of problem.

🟡 Robustness / maintainability

4. No validation of the version input format — malformed input degrades to a silent no-op.
The sed expressions only match [0-9]+\.[0-9]+\.[0-9]+. If someone dispatches with a typo or an unexpected format, the seds may match nothing and the resulting commit could contain partial/incorrect edits (the portfile REF substitutes the tag verbatim regardless). Consider an early guard, e.g. [[ "$ver" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]] || { echo "bad version"; exit 1; }, so a bad input fails fast rather than opening a subtly wrong PR.

5. Re-publishing an existing version makes the sanity check misleading.
The Python does if not any(e.get('version') == ver ...): insert(...), so re-running for an already-listed version leaves versions/h-/hsm-collector.json untouched — but the port files (and thus the git-tree) are re-committed. The "Sanity-check version DB git-tree" step then compares HEAD:ports/hsm-collector against the old recorded git-tree via head -1; it will fail (correctly flagging drift) but with an opaque message. Worth an explicit "version already present" branch that either updates the git-tree or aborts with a clear message. Also note port-version in baseline.json is never reset/bumped — fine for new versions, but a re-publish that needs a port fix would require manual intervention.

6. git push -f unconditionally force-pushes the registry/hsm-collector-<ver> branch.
Harmless in the normal single-run case, but combined with gh pr create ... || echo "may already exist", a second run silently rewrites an in-review branch. Acceptable given the naming scheme; just calling it out.

Minor

• sed uses | as its delimiter; the substituted ${tag}/${sha} are safe today, but if the input validation in (4) isn't added, a value containing | or & would corrupt the replacement.
• Consider actions/checkout@v4 is pinned to a tag rather than a SHA — fine for a first-party workflow, but SHA-pinning is the hardened convention for actions with contents: write.

Overall the core registry-update logic is sound; items 1 and 3 are the ones I'd fix before merging (the empty-hash guard bug and the input interpolation), and 2 is worth confirming so the "check passes" instruction is actually enforceable.

[github-actions]

PR review: hsm-collector-registry-publish.yml

Overall this is a well-structured, security-conscious workflow. It automates a fiddly manual process (bump port REF/SHA512 + version DB + validate + PR) and the comments explain the non-obvious steps clearly. The injection-hardening is genuinely good: dispatch input goes through env: rather than ${{ }} interpolation into the shell body, and ver is regex-validated before it's ever used to build a tag or path. Nice work.

Below are the issues I found, none blocking, roughly in order of importance.

1. baseline.json port-version is never reset — latent inconsistency

The Python step updates only baseline and leaves port-version untouched:

b['default']['hsm-collector']['baseline'] = ver

Meanwhile the new version-DB entry ({"version": ver, "git-tree": tree}) has an implicit port-version of 0. Today the baseline port-version is 0 and this workflow never bumps it, so the two stay consistent. But if port-version is ever set to a non-zero value (manually, or by a future change), publishing a new version would leave the baseline pointing at version=X.Y.Z, port-version=N while the DB only has port-version=0 for that version — and vcpkg resolution would break. This is exactly the kind of bookkeeping vcpkg x-add-version handles for you; the hand-rolled JSON edits diverge from it. Consider explicitly setting b['default']['hsm-collector']['port-version'] = 0 when publishing a new upstream version to make the invariant self-enforcing.

2. gh pr create ... || echo masks all failures, not just "already exists"

gh pr create ... || echo "A PR for $BR may already exist."

The branch has already been force-pushed at this point, so if gh pr create fails for a real reason (auth/permission/rate-limit/API error), the step still exits 0, the run is reported green, and no PR exists — a silent failure that's easy to miss. Prefer distinguishing the "already exists" case, e.g. check gh pr list --head "$BR" first, or grep stderr for the duplicate-PR message and re-exit 1 on anything else.

3. Re-publishing an already-merged version rewrites its git-tree

The "idempotent" logic drops and re-inserts the entry for ver:

d['versions'] = [e for e in d['versions'] if e.get('version') != ver]
d['versions'].insert(0, {"version": ver, "git-tree": tree})

This is correct and helpful for re-running before merge. But vcpkg treats a published version+port-version → git-tree mapping as immutable. If someone re-runs the publish for a version that was already merged (e.g. the source tag was force-recreated with different content, so the SHA512/tree changes), this would silently rewrite history for a version consumers may already have pinned. Worth a guard that refuses to change the git-tree of a version already present on master, or at least a ::warning:: when the recomputed tree differs from an existing one.

4. No concurrency group — racing publishes

Two collector-v* tags pushed close together (or a push + a manual dispatch) run concurrently, both branch from master, and can open competing PRs / clobber each other's force-pushed branches. Adding a concurrency: group (e.g. keyed on the workflow name) would serialize them.

Minor / nits

• Show the computed change / build validation only exercise the overlay port, not the registry version-DB resolution path. The PR body already discloses that the separate hsm-collector-registry check won't auto-run, so this is a known gap — just calling it out: a broken versions/ entry (e.g. wrong git-tree) would not be caught by this run, only the sanity-check grep/rev-parse step guards it.
• git push -f to registry/hsm-collector-${VER} is fine for an ephemeral automation branch, but combined with HSMCppWrapper #3 it means a re-run can rewrite an open PR's branch out from under a reviewer.
• The empty-stream SHA512 guard constant is correct (128 hex, matches sha512("")), and the curl -f + length check make it belt-and-suspenders — no issue, just confirming it checks out.

None of these block the PR; #1 and #2 are the ones I'd most want addressed since they can produce silent breakage.

[github-actions]

PR Review — hsm-collector registry publish workflow

Reviewed .github/workflows/hsm-collector-registry-publish.yml (the only file changed) against the port/version files it manipulates (ports/hsm-collector/portfile.cmake, vcpkg.json, versions/h-/hsm-collector.json, versions/baseline.json).

Overall this is a carefully written workflow. The security hygiene is genuinely good — untrusted inputs are passed through env rather than interpolated into shell bodies, the version string is validated against a strict X.Y.Z regex before use, curl -f avoids hashing a 404 body, the empty-SHA512 sentinel is a correct belt-and-suspenders guard, and the git-tree-immutability check plus the --overlay-ports build validation show real attention to vcpkg's model. The inline comments explain the non-obvious decisions well.

The findings below are mostly medium/low; nothing is a blocker.

---

1. No version-monotonicity check — baseline can silently roll backward (Medium)

Prepare registry update sets the registry baseline to whatever version is being published, unconditionally:

b['default']['hsm-collector']['baseline'] = ver
b['default']['hsm-collector']['port-version'] = 0  # reset on a new version

If someone re-publishes or fixes an older version (e.g. pushing collector-v0.6.1 after 0.6.3 is already the baseline, or running workflow_dispatch with an older version), the baseline is moved backward. Every downstream consumer that pins to this registry's baseline would then be silently downgraded, even though the versions/ array still contains the newer entries.

Consider comparing ver against the current baseline and only advancing it (or refusing/warning when the new version is not strictly greater). The code already has the natural place to do it — right next to the git-tree immutability check.

2. git push -f can clobber manual edits on an existing PR branch (Low)

git push -f -u origin "$BR"

The force-push is deliberate for idempotent re-runs, but if a reviewer has pushed a manual fixup commit onto the open registry/hsm-collector-<ver> PR branch, a re-run of this workflow will overwrite it without warning. Given the branch is bot-owned this is usually fine, but a --force-with-lease would fail loudly instead of silently discarding, which is safer.

3. GitHub auto-generated tarball SHA512 stability (Low / informational)

The SHA512 is computed from https://github.com/<repo>/archive/<tag>.tar.gz. GitHub's auto-generated source archives have historically had reproducibility issues (compression changes can alter the bytes and thus the hash), which would make a consumer's later vcpkg install fail hash verification even though the publish run validated fine. GitHub has committed to archive stability, so in practice this is low-risk today — but it's the reason many registries prefer release assets or git archive. Worth being aware of; no change strictly required.

4. Workflow must live on the trigger ref to fire on tag push (Low / informational)

on: push: tags: collector-v* will only trigger once this workflow file exists on the commit the tag points to. Since the file is currently only on this feature branch, tag pushes won't invoke it until the PR is merged to master and future tags are cut from commits that include it. Not a bug, but a deployment expectation worth calling out so the first post-merge tag isn't a surprise no-op.

---

Things I checked that are correct

• Shell injection surface — tag/ver/INPUT_VERSION flow through env and the semver regex guarantees the surviving TAG is exactly collector-v<semver>, so its later use in the archive URL and sed is safe. The Python heredoc uses <<'PY' (no shell expansion) and reads inputs via os.environ. Good.
• git-tree ordering — new entry is insert(0, ...) (newest-first, per vcpkg convention), and the head -1 in the sanity-check therefore reads the newly published tree, so recorded == actual holds.
• Commit sequencing — port is committed first, tree captured, then versions/ folded in via --amend; since touching versions/ doesn't change the ports/hsm-collector subtree, the recorded git-tree stays valid, including after a squash merge (git-tree is content-addressed).
• sed patterns — REF/SHA512/version regexes each match exactly one line in the current files; the empty-SHA512 constant matches the true sha512("") value.
• PR gating — if: github.event_name == 'push' || github.event.inputs.dry_run == 'false' correctly opens PRs on tag push and honors the dry-run default for manual runs; the existence check via gh pr view "$BR" avoids duplicate PRs.
• Permissions / concurrency — scoped to contents: write + pull-requests: write, and the non-cancelling concurrency group correctly serializes writes to the shared version DB.

[github-actions]

PR Review: ci/hsm-collector-registry-publish

This PR adds a single new workflow, .github/workflows/hsm-collector-registry-publish.yml, that automates publishing a new hsm-collector version into the repo's vcpkg registry (compute tarball SHA512 → update port + version DB → build/install to validate → open PR).

Overall this is a high-quality, carefully-reasoned workflow. Inputs are validated and passed through env (not interpolated into shell bodies), publishes are serialized via concurrency, the empty-stream SHA512 guard and the vcpkg immutability check are thoughtful, and the inline comments explain the non-obvious decisions well. I only have minor findings — nothing blocking.

Findings

1. gh pr view matches closed/merged PRs, so re-publish can silently skip PR creation (low)

In the Open PR step:

if gh pr view "$BR" --json number >/dev/null 2>&1; then
  echo "A PR for $BR is already open; pushed the refreshed branch to it."
else
  gh pr create ...
fi

gh pr view <branch> resolves the branch's PR regardless of state — if a PR from registry/hsm-collector-<ver> was previously merged or closed, this returns success, so no new PR is created and the message falsely claims one is "already open". The branch was still force-pushed, leaving it in an ambiguous state with no open PR. Since branch names are keyed on version and the immutability check refuses tree rewrites, this only bites a re-publish of an already-merged version, hence low severity. Consider scoping to open PRs:

if [ -n "$(gh pr list --head "$BR" --state open --json number --jq '.[].number')" ]; then

2. ${{ github.repository }} is interpolated into the shell body, contradicting the file's own convention (low)

The Compute source tarball SHA512 step explains (in the Resolve + validate version step) "never interpolate ${{ }} into the shell body," yet does:

url="https://github.com/${{ github.repository }}/archive/${TAG}.tar.gz"

github.repository is a trusted value (not attacker-controllable), so this is not an actual injection risk — but it's inconsistent with the stated rule and worth moving to env: (like TAG) for consistency and to keep the convention airtight. Note the same ${{ github.repository }} also appears in the ::error:: message string in that step.

3. Port build is validated, but registry git-tree resolution is not exercised end-to-end (informational)

The Validate the updated port builds + installs step uses --overlay-ports=ports, which bypasses the version database — it proves the refreshed REF + SHA512 resolve and the port builds, but does not test that a consumer resolving hsm-collector through the registry (baseline + versions/h-/hsm-collector.json git-tree) actually works. The Sanity-check version DB git-tree step does textually confirm the recorded git-tree equals git rev-parse HEAD:ports/hsm-collector, which is a solid guard, and the PR body honestly flags that the separate hsm-collector-registry check won't auto-run (GITHUB_TOKEN limitation). This is a documented, acceptable gap — noting it so reviewers know the registry-resolution path is only checked structurally, not by an actual install through the registry.

Things verified as correct

• The tarball URL (/archive/<tag>.tar.gz) matches what vcpkg_from_github fetches, so the computed SHA512 will match at install time.
• The empty-string SHA512 constant is correct and 128 hex chars; combined with curl -f it robustly rejects a missing tag.
• Recording git rev-parse HEAD:ports/hsm-collector as the git-tree replicates vcpkg x-add-version correctly; because git trees are content-addressed, the recorded tree survives a squash/rebase merge as long as the port files land identically.
• Version regex validation (^[0-9]+\.[0-9]+\.[0-9]+$) on the tag/input closes the shell-injection surface for the dispatch input.
• Baseline "advance-only" logic, immutable-git-tree refusal, and idempotent re-insertion of the version entry are all sound; the indent=2 JSON output plus trailing newline matches the existing file formatting.
• concurrency (group without version, cancel-in-progress: false) correctly serializes all publishes against the shared version DB.
• The sed expressions target the single REF/SHA512/"version" occurrences and won't collide with dependency version>= constraints.

Nice work — recommend addressing finding #1 (and optionally #2) before merge.

[github-actions]

Review — hsm-collector-registry-publish workflow

Overall this is a well-constructed automation workflow. The security hygiene is genuinely good: dispatch input and tag name are passed through env: rather than interpolated into the shell body (with a comment explaining why), the version is regex-validated before use, the Python heredoc is single-quoted (<<'PY') so no shell expansion happens inside it, and the curl -f + empty-SHA512 guard correctly prevents a 404 from being hashed as valid content. The git-tree-is-immutable refusal and the "only advance the baseline" logic show careful thought. Actions and the vcpkg commit are pinned. Nice work.

A few findings, none blocking:

1. Idempotent re-run of the current version silently resets port-version (medium)

if semver(ver) >= semver(cur):
    b['default']['hsm-collector']['baseline'] = ver
    b['default']['hsm-collector']['port-version'] = 0  # reset on a new version

The guard uses >=, so re-publishing the version that is already the baseline (same ver == cur, same git-tree, so the immutability check does not trip) falls into this branch and unconditionally forces port-version back to 0.

Scenario: 0.6.2 is the baseline, later someone lands a port-only fix and bumps baseline.port-version to 1 (a normal vcpkg workflow, done outside this automation). If this workflow is then re-triggered for 0.6.2 (accidental re-tag, or a manual dispatch), it silently rolls port-version back to 0, regressing what consumers resolve. Consider only resetting when the version strictly increases, and leaving port-version untouched when ver == cur:

if semver(ver) > semver(cur):
    b['default']['hsm-collector']['baseline'] = ver
    b['default']['hsm-collector']['port-version'] = 0
elif semver(ver) == semver(cur):
    b['default']['hsm-collector']['baseline'] = ver  # keep existing port-version

2. Validation never exercises the http feature (low — coverage)

"$VCPKG_ROOT/vcpkg" install hsm-collector --overlay-ports=ports

This installs the base port only. The http feature (which pulls curl[ssl] and defines HSM_COLLECTOR_HTTP) is the part most likely to break on a version bump, yet the "known-good" claim in the PR body is based solely on the featureless build. Consider install hsm-collector[http] (or hsm-collector[core,http]) so the PR is validated on the code path most consumers actually use. This also matches the intent stated in the step comment ("proves the refreshed SHA512 + REF actually resolve and the collector builds").

Also note the validation runs on ubuntu-latest, while the companion hsm-collector-registry.yml consumer check is Windows/MSVC. Because the auto PR is opened by GITHUB_TOKEN, that Windows check won't auto-run (correctly documented in the body), so the Windows path stays entirely unverified until someone re-runs it manually. Worth keeping in mind if the collector has platform-specific surface.

3. Python relies on refcount GC to flush files before reopening (low — robustness)

json.dump(b, open('versions/baseline.json', 'w'), indent=2); open('versions/baseline.json', 'a').write('\n')

The write-mode handle from open(...,'w') is never explicitly closed; the subsequent open(...,'a') assumes it has already been flushed and closed. This works under CPython's reference counting (which GHA uses), but it's fragile — on any non-refcounting interpreter the append could interleave with an unflushed buffer. A with block, or writing the trailing newline into the same handle, would be safer and clearer. Same pattern appears for versions/h-/hsm-collector.json.

4. git push -f on the publish branch (low)

git push -f -u origin "$BR"

The force-push is reasonable for a bot-owned, deterministically-named branch and supports the "refresh an already-open PR" case. Just flagging that if anyone ever pushes a manual commit onto registry/hsm-collector-<ver>, it will be clobbered without warning. Given the naming convention this is acceptable; a --force-with-lease would be marginally safer.

5. Concurrent publishes can still collide at merge time (informational)

The concurrency group correctly serializes runs so they can't race on the runner. But two publishes for different versions each branch from master and independently prepend to versions/h-/hsm-collector.json and rewrite baseline.json. If both PRs are open before either merges, the second merge will hit a conflict in those files that a human must resolve. Not a workflow bug — just worth documenting so maintainers aren't surprised.

Minor notes

• grep -oiE '[0-9a-f]{40}' ... | head -1 in the sanity-check relies on the new entry being prepended (insert(0, ...)) so head -1 picks the freshly added git-tree. This holds today and matches the companion workflow, but it's an implicit coupling between the Python insert order and the grep — a comment would help.
• The SHA512 approach depends on GitHub's /archive/<tag>.tar.gz bytes being stable, which is the same assumption vcpkg_from_github makes, so this is consistent by design. No action needed; noting it as the one external assumption the whole flow rests on.

Verdict

No correctness bugs that block the happy path; the git-tree/commit/amend sequencing and the version-DB invariant are handled correctly and match the companion hsm-collector-registry.yml check. Finding #1 is the one I'd address before merge; the rest are quality/robustness improvements.

[github-actions]

PR #1268 Review — hsm-collector-registry-publish workflow

This is a well-crafted, defensively-written workflow. The single new file (.github/workflows/hsm-collector-registry-publish.yml) automates what was previously a manual vcpkg-registry publishing recipe. The security posture is notably good: dispatch inputs flow through env rather than ${{ }} interpolation into the shell body, the version is validated against a strict semver regex before use, the SHA512 download uses -f + an empty-hash guard, and there is no untrusted pull_request_target trigger. The git-tree bookkeeping (commit port → capture git-tree → amend versions/, relying on the content-addressed subtree being unaffected by the amend) is correct and the inline comments are excellent.

Findings below, roughly ordered by importance. None are blocking; most are robustness/accuracy notes.

1. "Known-good" claim overstates confidence — validation is Linux-only, consumers are Windows (medium)

The Validate the updated port builds + installs step runs on ubuntu-latest, but the companion hsm-collector-registry.yml consume check and the actual downstream consumers target Windows/MSVC (that workflow is explicitly "Windows-only: the aggregator target is Windows/MSVC"). The generated PR body then asserts:

The updated port was built + installed in the publish run … so it is known-good.

A Windows-only build break (Schannel vs OpenSSL feature wiring, MSVC-specific code, path handling) would pass this Linux validation, be merged as "known-good," and — as the PR body itself notes — the Windows registry check will not auto-run on a GITHUB_TOKEN-opened PR. Consider either running the validation on windows-latest to match the real consumer, or softening the PR-body wording to "built + installed on Linux; run the Windows hsm-collector-registry check before merging."

2. Concurrent open PRs will conflict in versions/ (low–medium)

The concurrency group serializes workflow runs, but a run finishes when the PR is opened, not merged. If two versions are published before the first PR merges, both runs branch from the same (unmerged) master, both insert(0, …) into versions/h-/hsm-collector.json, and both edit versions/baseline.json — so the second PR to merge will hit a merge conflict requiring manual rebase. This is inherent to the open-a-PR model and is an acceptable trade-off, but it's worth a line in the header comment so an operator isn't surprised.

3. No retry on the source download (low)

curl -fsSL "$url" -o source.tgz

A transient 5xx/network blip while fetching the archive fails the whole publish. Adding --retry 3 --retry-connrefused (and optionally --retry-delay) makes the run more robust without weakening the correctness guards, since the SHA512 length/empty checks still validate the result.

4. Sanity-check parses JSON with grep/head instead of a JSON tool (low)

recorded=$(grep -oiE '[0-9a-f]{40}' versions/h-/hsm-collector.json | head -1)

This is correct today only because the new entry is prepended (newest-first) and the Python writer preserves that order — a coupling the comment does acknowledge. It's brittle against any future reordering or formatting change in the version DB. Since python3/jq are already available, reading the entry for $VER explicitly (e.g. the git-tree of the object whose version == $VER) would be self-documenting and order-independent. The same pattern is duplicated from hsm-collector-registry.yml, so this is a pre-existing convention rather than a new problem.

5. Same-version republish with changed port content is hard-blocked (low, likely by design)

The immutability guard refuses to rewrite a published version whose git-tree differs:

if existing and existing.get('git-tree') != tree:
    error("… refusing to rewrite … Bump the version.")

This correctly enforces vcpkg's version→git-tree immutability, but it also means a legitimate port-only fix (e.g. adding a CMake option or fixing the portfile without a source change) cannot be published through this workflow — it would need a port-version bump, which this workflow only touches in baseline.json, never in the versions/ entry. Given the baseline logic already reasons carefully about port-version, consider either supporting a port-version bump path or documenting in the header that port-only changes must be done manually.

6. GitHub archive tarball SHA512 stability (informational)

The SHA512 is computed over https://github.com/<repo>/archive/<tag>.tar.gz. GitHub does not contractually guarantee byte-stable generated archives, and past regenerations have invalidated stored SHA512s across the vcpkg ecosystem. This is not a defect introduced by this PR — vcpkg_from_github fetches the same URL, so the computed hash matches what the port downloads, and they'd break together. Flagging only so it's a known operational risk, not a surprise.

Nits

• permissions, concurrency: cancel-in-progress: false, and the dry_run default (true, so workflow_dispatch is safe-by-default) are all sensible choices — nice.
• The run-vcpkg commit pin (6f29f12e82a8293156836ad81cc9bf5af41fe836) matches hsm-collector-registry.yml, keeping the two lanes on the same vcpkg baseline. Good to keep them in sync going forward.

Verdict

Solid, security-conscious automation with correct git-tree handling. The main thing worth acting on before relying on it is finding #1 — either validate on Windows or align the "known-good" wording with what was actually tested. The rest are robustness/clarity improvements.

[github-actions]

Review: hsm-collector-registry-publish workflow

Reviewed .github/workflows/hsm-collector-registry-publish.yml (the only change in this PR) against ports/hsm-collector/* and versions/*.

Overall: This is a high-quality, carefully-reasoned workflow. The security posture is notably good — dispatch input flows through env: and is never interpolated into a shell body, the version is regex-validated before use, and the tarball is downloaded with curl -f (so a missing tag 404s instead of hashing an empty stream) plus a defensive length/empty-hash check. The version-DB immutability guard, the "compute the ports tree before folding in versions/, then sanity-check it" pattern, and the concurrency serialization all show real understanding of vcpkg registry semantics. No blocking bugs found. The notes below are minor.

Suggestions (non-blocking)

1. fetch-depth: 0 is unnecessary and slows every run.

- uses: actions/checkout@v4
  with:
    ref: master
    fetch-depth: 0

The job only creates a branch from master HEAD, commits, computes git rev-parse HEAD:ports/hsm-collector, and force-pushes — none of which need history. A full-history checkout of this repo is a meaningful cost per run for no benefit; fetch-depth: 1 is sufficient. (Only bump it if you later add history-dependent logic.)

2. git push -f silently clobbers manual commits on the auto-PR branch.
The idempotent re-run design (drop + prepend the versions/ entry, --amend) is sound, but combined with push -f it means if a maintainer pushes a fix commit onto an open registry/hsm-collector-<ver> PR (e.g. to hand-add a port-version or patch the build) and the workflow re-runs, that work is overwritten with no warning. The branch is clearly automation-owned, so this is acceptable — worth a one-line caution in the header comment so a maintainer isn't surprised.

3. GitHub auto-generated archive checksums are not guaranteed stable over time.
The SHA512 is computed from https://github.com/<repo>/archive/<tag>.tar.gz, matching what vcpkg_from_github fetches — so this is consistent with vcpkg norms and is validated at publish time. Be aware, though, that GitHub has historically regenerated /archive/ tarballs with different compression, changing the SHA512 and breaking pinned vcpkg ports across the ecosystem. If long-term reproducibility matters, consider attaching an immutable release asset per collector-v* tag and hashing that instead. Informational, not a defect in this PR.

4. versions/ entry omits port-version.
The inserted entry is {"version", "git-tree"} only, which implies port-version: 0. That's correct for the automated fresh-source path this workflow targets, and the header comment already scopes out port-only bumps. Just flagging that a same-version republish after a manual port-version bump would record an entry inconsistent with baseline.json's port-version — the ver == cur branch leaves the baseline port-version alone but still prepends a port-version: 0 versions entry. Given the documented scope this is an edge case, but a guard/warning when cur's port-version != 0 would harden it.

5. Minor robustness: the Python block does b['default']['hsm-collector'].get('baseline', ...), assuming the hsm-collector key already exists in baseline.json. True today, but a brand-new port (or a hand-edited baseline) would KeyError rather than emit a clear ::error::. A setdefault or an explicit check would give a friendlier failure.

Things I verified are correct

• github.event.inputs.dry_run == 'false' compares against the string 'false' — correct, since dispatch inputs are always strings even when typed boolean.
• The sed delimiters (|) and replacement text are safe: VER/TAG/SHA are constrained to [0-9a-f.] / validated semver, so no delimiter or backreference collisions.
• The id: v regex correctly rejects non-X.Y.Z versions before any of them reach a shell or filesystem operation.
• Concurrency group serializes publishes so two tag pushes can't race the shared version DB; cancel-in-progress: false is the right choice here.
• The git-tree recorded in versions/ is content-addressed, so it survives a squash/merge of the PR as long as the ports/hsm-collector content is unchanged — standard vcpkg registry behavior.

[github-actions]

PR #1268 Review — hsm-collector registry publish workflow

Reviewed git diff origin/master...origin/ci/hsm-collector-registry-publish (one new file: .github/workflows/hsm-collector-registry-publish.yml), cross-checked against the current ports/hsm-collector/, versions/h-/hsm-collector.json, and versions/baseline.json.

Overall this is a carefully-built, well-documented workflow. Shell injection is handled correctly (dispatch inputs flow through env:, never interpolated into ${{ }} in the shell body), the SHA512 computation is guarded (curl -f + download-to-file + empty-hash check), publishes are serialized with concurrency, and the port is built+installed before the PR is opened. The inline comments explaining the non-obvious decisions are genuinely good. Findings below are mostly low-severity edge cases and hardening suggestions.

Correctness / logic

1. (Low, latent) versions/ dedup keys on version only, not (version, port-version)
In the Prepare registry update Python step:

d['versions'] = [e for e in d['versions'] if e.get('version') != ver]
d['versions'].insert(0, {"version": ver, "git-tree": tree})

This drops every entry matching the source version and re-inserts a single port-version-0 entry. If a version X had previously received a manual port-only bump (a versions/ entry with port-version: 1 and a different git-tree), re-running publish for source version X would delete that X#1 entry from the version database — contradicting the stated intent "a manual port-only bump must not be clobbered". In practice the immutability guard above it usually catches this (the X#1 entry's git-tree differs from the freshly-computed tree → the step errors out), but only if that entry happens to be the first version == X match returned by next(...). The protection is therefore order-dependent. Consider making the dedup match on (version, port-version) so a genuinely older/other entry is never silently removed.

2. (Low) Backport publish rewrites master's HEAD port to the older version
The branch is created from master and sed rewrites REF/SHA512/vcpkg.json version unconditionally. Publishing an older version (the acknowledged backport case) will, on merge, leave master's portfile.cmake and vcpkg.json pointing at the older version. Consumers are unaffected because baseline resolution goes through the version DB's git-tree, and the header comment does cover the baseline-doesn't-roll-back behavior — but it's worth noting the HEAD port files themselves get stomped to stale content in that path. A guard that refuses (or warns loudly) when ver < cur before editing ports/ would make the backport story cleaner.

Robustness / hardening

3. (Low) curl has retries but no timeouts. curl -fsSL --retry 3 --retry-connrefused has no --connect-timeout/--max-time, so a slow/hung endpoint stalls the job until the (default 6h) job timeout rather than failing fast. Adding --max-time (and optionally a timeout-minutes: on the job) bounds the failure.

4. (Low) Version-DB JSON is hand-serialized. json.dump(..., indent=2) + trailing newline matches the current files, but it isn't guaranteed to match vcpkg's canonical x-add-version formatting/ordering, and the workflow never runs x-add-version --verify (it does its own tree comparison instead). This is fine functionally, but a future x-add-version run may reformat these files. Not blocking.

Nice touches worth calling out

• Injection-safe handling of github.event.inputs.* via env:.
• Download-to-file instead of piping into sha512sum (avoids hashing a 404 body), plus the empty-hash sentinel guard.
• Immutable version→git-tree guard that refuses to rewrite a published version.
• The independent Sanity-check version DB git-tree step (order-independent lookup).
• concurrency serialization and dry_run default true for manual dispatch.
• Building/installing the port before opening the PR, and the explicit note that the Windows/MSVC consumer check must be run manually.

Verdict

No blocking bugs found. The two correctness items are edge cases (re-publishing a manually-bumped version; backports) that are partly mitigated already; addressing #1 (dedup on (version, port-version)) and adding --max-time (#3) would be the highest-value follow-ups.

[github-actions]

PR Review: hsm-collector-registry-publish workflow

Reviewed the single new file .github/workflows/hsm-collector-registry-publish.yml against the files it mutates (ports/hsm-collector/portfile.cmake, ports/hsm-collector/vcpkg.json, versions/baseline.json, versions/h-/hsm-collector.json).

Overall: this is a high-quality, well-thought-out workflow. The header comment honestly documents its scope and limitations, the shell is set -euo pipefail throughout, inputs are handled safely, and the tricky vcpkg version-database mechanics (immutable git-tree, baseline-only-advances, idempotent re-runs) are handled correctly. Below are a few notes, none blocking.

Security — looks good ✅

• No command injection. Untrusted values (github.event.inputs.version, github.event_name, github.repository) are passed through env: and referenced as $VAR rather than interpolated as ${{ }} into the shell body — the correct pattern, and it's even called out in a comment.
• Input is constrained before use. For workflow_dispatch, INPUT_VERSION is validated against ^[0-9]+\.[0-9]+\.[0-9]+$; for push, tag=$GITHUB_REF_NAME is only written to $GITHUB_OUTPUT after the derived ver passes the same regex, so a hostile tag like collector-v1.2.3;evil fails closed. Downstream TAG/SHA/VER are therefore guaranteed to be shell-safe (semver / lowercase hex) before being spliced into the curl URL and sed replacements.
• Permissions are scoped to contents: write + pull-requests: write, both genuinely required (branch push + PR create).
• The heredoc uses quoted <<'PY', so no shell expansion leaks into the Python; values arrive via os.environ.

Correctness — verified ✅

• The sed patterns match exactly one target each in the current files (confirmed: REF collector-v0.6.2, the 128-hex SHA512, and the single "version": "0.6.2" line). The collector-v mention in the portfile comment is not prefixed with REF , so it is not touched.
• The git-tree is computed as git rev-parse HEAD:ports/hsm-collector after committing the port but before folding in versions/, and the versions/ change is --amended in (which cannot alter the ports/ subtree) — so the recorded tree matches the port content. The "Sanity-check" step independently re-verifies recorded == actual. Since tree SHAs are content-addressed, the value survives squash/rebase merges, so consumers can still resolve it. Correct.
• Baseline logic (advance-only; reset port-version on a strictly-newer version; warn on older/backport; refuse to rewrite an already-published version with a different tree) is sound.
• SHA512 guard: downloading to a file with curl -f (so a 404 → non-zero exit) instead of piping into sha512sum, plus the length-128 and empty-hash checks, is the right defense.

Minor / low-priority

1. Stale comment referencing a mechanism that no longer exists. In the Python block, the "newest-first" comment says the ordering is relied on by "the sanity-check step's grep ... | head -1". The actual Sanity-check step uses an order-independent Python next(... if e['version']==VER) lookup — there is no grep | head -1. The ordering (prepend newest-first) is still a fine convention, but the justification is misleading and will confuse a future maintainer. Suggest updating the comment to say the newest-first order is just a display/readability convention.

2. GitHub auto-generated tarball checksum stability (known ecosystem risk, not a defect in this PR). The pinned SHA512 is computed from https://github.com/<repo>/archive/<tag>.tar.gz. GitHub does not contractually guarantee byte-stable auto-generated archives; historically a change in GitHub's git/gzip has invalidated such checksums, which would break vcpkg install for consumers with a hard-to-diagnose hash mismatch. This is inherent to vcpkg_from_github (the whole vcpkg ecosystem shares the risk), so it's not something to fix here — just worth being aware of as an operational failure mode. Not blocking.

3. The tarball is downloaded twice — once to compute the SHA512, once again by vcpkg during the "Validate the updated port builds" step. Negligible cost for a publish workflow; fine to leave.

4. DB drift across concurrent open PRs is documented but not enforced. The header notes that two publishes opened before either merges "will conflict in versions/ (rebase the second)," because each run re-checks-out master (fetch-depth: 1) and so the second branch's versions/h-/hsm-collector.json omits the first's not-yet-merged entry. Honest and acceptable given the concurrency group serializes runs; just be aware the git-level conflict is the safety net, so don't force-merge the second PR without rebasing.

Verdict

No blocking issues. Security handling is exemplary and the vcpkg mechanics are correct. Recommend fixing the stale grep | head -1 comment (item 1) before/after merge; the rest are informational.