Skip to content

fix(update): select release asset by exact name, not prefix#201

Merged
jahwag merged 2 commits into
jahwag:mainfrom
clauderesearch:fix/update-asset-exact-match
Jun 10, 2026
Merged

fix(update): select release asset by exact name, not prefix#201
jahwag merged 2 commits into
jahwag:mainfrom
clauderesearch:fix/update-asset-exact-match

Conversation

@clauderesearch

Copy link
Copy Markdown
Collaborator

Closes #194.

runUpdate matched the release asset with strings.HasPrefix and broke on the first hit, so selection depended on GitHub's unguaranteed asset ordering. Any sibling asset whose name starts with clem_<os>_<arch> would be downloaded and renamed over the running binary.

Change

  • Extract selection into selectBinaryAsset and match the asset name exactly (==) against clem_<os>_<arch>.
  • Name the asset struct (ghAsset) so the selector is testable; no other callers exist.
  • Table-driven tests: prefix siblings ordered first, prefix-only sets, wrong arch, empty list, and a fixture mirroring the actual v0.12.1 release asset layout.

Notes

  • Adversarial review ran (refute-style, against .goreleaser.yaml and the live v0.11.0–v0.12.1 release asset lists). It confirmed exact match is correct for every current/historical layout (linux-only builds, binary archive format, no .exe case) and caught one inaccuracy in my draft rationale: the published SBOM embeds the version (clem_0.12.1_linux_amd64.sbom.json), so it does not share the binary prefix today — the live risk is narrower (.sig/.pem siblings or a future SBOM naming change). Comments and this body were corrected accordingly.
  • Checksum verification of the downloaded asset is deliberately out of scope — that is bug(update): clem update does not verify binary checksum — silent replacement on MITM or compromised release #126.

HasPrefix with break-on-first-hit made asset selection dependent on
GitHub's unguaranteed asset ordering: any sibling asset starting with
clem_<os>_<arch> would be downloaded, chmod 0755'd, and renamed over
the running binary. Match the exact name instead and extract the
selection into selectBinaryAsset for testability.
@clauderesearch clauderesearch requested a review from jahwag as a code owner June 10, 2026 13:37
@jahwag jahwag enabled auto-merge (squash) June 10, 2026 20:26
@jahwag jahwag merged commit 260628a into jahwag:main Jun 10, 2026
9 checks passed
jahwag pushed a commit that referenced this pull request Jun 10, 2026
v0.12.2 landed three fixes (#198, #199, #201) that overlap this branch.
Resolutions defer to upstream's reviewed decisions where we collided:

- config: adopt #198's control-char-only validation for name AND role
  (drop this branch's stricter metachar rejection — upstream's policy is
  escape-at-render-site, shipped for the alert sink in #199 and tracked
  for the runner templates as #112). Keep this branch's modelRe check,
  which has no upstream equivalent.
- update: adopt #201's selectBinaryAsset exact-name match wholesale (it
  also fixes the asset-ordering hazard this branch's refactor had
  preserved); keep this branch's latestReleaseURL test seam and
  fetchLatestRelease status tests alongside upstream's table tests.
- runner: #199's escapeForAlert merged cleanly with the gofmt fixes.

Full suite green under C and UTF-8 locales; lint clean.

https://claude.ai/code/session_013fHtnLBPf9XGX3pemrAwL5
Signed-off-by: Claude <noreply@anthropic.com>
clauderesearch added a commit that referenced this pull request Jun 10, 2026
…efresh) (#205)

Rebases the skills feature (059e5bf + 822c997, previously only on the
v0.10.0-snapshot.1 channel) onto current main, restoring per-provision
and per-iteration team-skills sync that went dormant after the box moved
to mainline v0.13.0. Closes #204.

## What
- Top-level skills_repo config key: clem provision clones the repo per
agent and symlinks shared/<skill> and <agentKey>/<skill> into
~/.claude/skills/; idempotent re-runs git pull --ff-only, stale symlinks
pruned.
- clem sync-skills subcommand + runner hook: skills refresh at the top
of every iteration, no operator round-trip after a skills PR merges.
- clem update --snapshot flag: opt-in prerelease channel (goreleaser
prerelease: auto keeps snapshot tags off stable hosts).

## Rebase conflict resolutions (vs v0.13.0-era main)
- config.go: SkillsRepo registered as a real struct field, so it passes
the new strict unknown-key validation; isPlausibleGitURL check runs in
Load().
- IsValidExtensionName moved to extensions.go next to extensionNameRe
(file was split since the original commits).
- update.go: kept main's exact-name selectBinaryAsset (#201) and
test-overridable URL vars; added Prerelease/Draft fields +
allReleasesURL for the snapshot channel.
- runner.go/provision.go: skills hooks re-inserted into the refactored
provisionAgent / Params paths alongside ProxyExport/SidecarServers.

## Verification
- go build ./... clean, gofmt clean, go vet clean
- go test ./... all packages ok, including restored skills tests:
TestLoad_SkillsRepoAccepted/Rejected,
TestGenerate_SkillsSyncInjectedWhenRepoSet/AbsentWhenRepoUnset,
SyncSkillsRepo manager tests
- Pre-push secret-scan flag is a false positive: neither commit diff
contains a GH_TOKEN read (grep of both diffs is empty; provision.go:46
is pre-existing main code), pushed with CLEM_HOOK_SKIP_CODE_SCAN=1

Release plan per jahwag: merge, then tag v0.14.0 (new feature = minor
bump rather than v0.13.1).

---------

Co-authored-by: jahwag <540380+jahwag@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security(update): release-asset match by HasPrefix can pick the SBOM instead of the binary — wrong-artifact self-replacement

2 participants