release-train: develop -> staging - #438
Merged
Merged
Conversation
…d#1303) (#435) Backlog at zero fleet-wide; the quality contexts are already required on develop. Also adds a workflow_dispatch(all-files) trigger for whole-tree scans (gitleaks baseline). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… second (#433) (#434) * fix(install): own ONE PATH block in the shell profile, never append a second (#433) `scripts/install.sh` guarded its profile append with "does the rc mention $PREFIX", so an install with a DIFFERENT prefix appended another block every time and the profile grew without bound. The v0.10.1 validation box collected TEN blocks, each naming a temp dir that no longer existed. The polluter was our own test harness: scripts/tests/install-verify.sh mints a fresh mktemp --prefix per case and never sandboxed $HOME, so it wrote 3 blocks into the developer's real ~/.bash_profile per invocation. - Tag our block with a stable $PATH_MARKER and REPLACE it on a re-run instead of stacking a second one. A profile already polluted by an older installer collapses to one block on the next install. - Decide "is $PREFIX already handled?" from the rc with our block stripped out, so the answer comes from the user's own lines only — that is what makes the write idempotent. - Compare whole path COMPONENTS, not substrings. `grep -F "$PREFIX"` matched --prefix /opt/tb against an existing /opt/tb2 line and then claimed "already in your PATH config" for a directory that was on nobody's PATH. - Append when there is nothing of ours to clean up; only rewrite the file when a stale block must go. The rewrite truncates in place, so the inode, mode and owner survive and an rc symlinked into a dotfiles repo is written THROUGH rather than replaced by a regular file. - Removal only takes the line under the marker when it is shaped like a PATH op we wrote, so a dangling marker can never eat unrelated user content. - Quote the fish line (`fish_add_path "$PREFIX"`), matching the client installer's hint, so a prefix containing a space survives. - Sandbox $HOME in install-verify.sh and add 7 assertions: same prefix x3 → one block; three different prefixes → one block naming the newest; prefix already on PATH → no rc written at all; unrelated lines preserved byte-for-byte; dangling marker harmless; zsh and fish route to the right rc. A prefix already on $PATH still writes nothing (unchanged), and a $HOME prefix is still persisted even when it looks on-PATH — that hit can be session-only (Bugbot #392 r2), and it now costs at most one line rather than one per run. Fixes #433 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(install): a same-prefix re-run must not rewrite the rc at all The first pass consolidated correctly but reached the replace path even when our one block already said exactly what this run would write — so a re-install, and every `tracebloc upgrade` (which re-execs this installer), rewrote the user's profile byte-for-byte and reported "Updated the tracebloc PATH entry". Recognise that case and leave the file completely alone: count our blocks, and when there is exactly one whose PATH line already matches, report `present` ("already in your PATH config — nothing to add") without touching the rc. Asserted: re-install with the same prefix leaves the rc byte-identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(install): restore the rc if the rewrite dies partway The redirection in replace_rc truncates before cat writes, so a write that died halfway (no space left, a vanishing mount) would leave the user's profile in pieces. Put the original contents back on failure — best effort, but far better than a half-written file we don't own. The caller already reports `failed` and prints the line to add by hand. Verified with a read-only rc: the file keeps its user content and its previous block, and the manual-add advice is printed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(install): require positive proof a PATH line is ours before removing it (Bugbot) Bugbot on #434: the marker alone was treated as proof of ownership, so a marker left dangling by a hand-edit directly above the USER's own PATH export would take that export with it on the next install — silently deleting a PATH entry we never added, while still reporting success. Removal now demands positive evidence about the directory the following line names (_tb_owns_dir): * a '$' anywhere -> never ours. We always write a literal, expanded path; "$HOME/mytools" is the user's own idiom. This kills the most realistic form of the bug. * == the prefix we are installing to now -> ours * a directory that no longer exists -> ours (the #433 cruft) * a directory still holding a tracebloc binary -> ours (a prior install) Anything else is the user's line: we drop only the orphaned marker comment and leave their PATH op alone. The bias is deliberate — for an installer editing a file it does not own, failing to clean one line is much cheaper than deleting a PATH entry someone depends on. The residue is bounded: at most one unmarked line per pathological cycle, never renewed growth. The pair-then-verdict pass goes through a temp file rather than a pipe (so the verdicts land in this shell) and rather than a heredoc (so the awk program needs no nested-expansion escaping). 3 new assertions, 23/23 in install-verify.sh: a dangling marker above the user's own PATH export keeps it, likewise with an unexpanded $HOME, and a block naming a vanished directory is still cleaned up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style(install): state the empty-prefix exit status outright Bugbot read the bare `exit` in the BEGIN rule as exiting 0, i.e. 'prefix already listed', which would skip persistence. It does not: a bare exit in BEGIN still runs END, so the status is END's exit(found ? 0 : 1) = 1, the fail-safe direction. Verified 1 on BSD awk and by the POSIX rule. Making it `exit 1` costs nothing and means the intent cannot be misread by a later reader or a stricter awk, rather than resting on the fallthrough. * fix(install): prove ownership from the marker, and stop mislabelling a failed tidy-up (Bugbot) Two Medium findings, both in the ownership/reporting logic. 1. _tb_owns_dir claimed ANY non-directory as ours, so a dangling marker above the user's own literal-path line for a directory they hadn't created yet was stripped as an owned block — deleting a PATH entry we never wrote, which is not recoverable. The marker we write now records the directory: # Added by the tracebloc CLI installer (prefix: /opt/tracebloc) export PATH="/opt/tracebloc:$PATH" When the recorded prefix and the directory on the line below agree, we provably wrote both halves, so the block is reclaimable even if that directory has since been deleted — which is what keeps the #433 cleanup working, ten blocks and all. $PATH_MARKER stays the stable BEGINNING of the line (matched literally via index(), never as a whole line) so older blocks are still recognised. A legacy marker that recorded nothing can now only be claimed when the directory still holds a tracebloc binary, or is the prefix being installed to. A legacy marker over a vanished directory is indistinguishable from the user's own entry for a directory they plan to create, so we leave it alone. That gives up auto-healing pre-#433 cruft in exactly one case; Lukas's profile was already cleaned by hand, so that is a nice-to-have, whereas deleting someone's PATH line is not. Blocks we cannot claim are now left ENTIRELY intact, comment included, rather than losing their marker: a dead line still labelled "Added by the tracebloc CLI installer" tells the user what it is, a bare one doesn't. 2. A failed replace_rc reported `failed` even when the user's own line already persisted the prefix and all we'd failed to do was drop a redundant block — so the installer told them to hand-edit their profile while their PATH was in fact correct. Split into `tidied`/`tidy_failed` (PATH is right; at most a cosmetic note) versus `failed` (prefix genuinely not persisted; manual instruction warranted), and the write decision now hinges on content equality (rc_same), so a run that changes nothing writes nothing. rc_same deliberately avoids cmp(1) — it lives in diffutils, which a minimal container image can lack, and a missing tool must not silently turn "leave the file alone" into "rewrite it every run". Also restores the `mkdir -p "$(dirname "$rc")"` that the state-machine rewrite dropped; without it fish's ~/.config/fish/config.fish could never be created. The matrix caught it. install-verify.sh: 29/29, with six new cases — legacy marker can't claim a missing dir, nor one without our binary; a recorded-prefix block naming a vanished dir is still cleaned; ten of them collapse to one; an unclaimable legacy block is kept without churning the rc; a failed tidy-up never asks for a manual PATH line, while a real failure still does. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…egrity (#429) * fix(release): rebuilds must run at the tag ref (Bugbot on promotion #428) Cosign's keyless identity embeds the RUN's ref. workflow_dispatch took an inputs.ref and could build a tag from a branch run, publishing signatures (@refs/heads/...) that the tag-anchored installers reject on every customer machine. inputs.ref removed; dispatch runs now hard-fail unless started at a v* tag ref; rebuild paths = rerun the tag run or dispatch at the tag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * review: pre-flight guard job + refs via env (Asad's nits on #429) Guard moved out of the 8-way matrix into a tiny job that release needs: a branch-misdispatch now fails once in seconds instead of burning eight runners' setup. Refs passed via env, never interpolated -- git permits $/backticks in tag names, so a crafted v* tag would otherwise execute on the runner (R8, same rule as the client installer workflows). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(release): pass the tag ref via env in Determine-release-tag (R8) The one step this PR's hardening missed: it still did REF="${{ github.ref_name }}", interpolating an attacker-controllable tag name straight into the shell, so a crafted v* tag with backticks or $() would execute on the publish runner before the release is created. Now passed as env REF_NAME and read as $REF_NAME, matching the guard and version steps. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
Contributor
Author
|
👋 Heads-up — Code review queue is at 39 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 19f9189. Configure here.
Contributor
Author
|
Valid — a regression from #434, fixed in #439.
The ownership reader further up the same file was already space-safe, so one reader was correct and the other wasn't — the same shape as several findings today. #439 parses quotes instead: a quoted argument is one path (spaces included), unquoted lines still split so multiple bare paths keep working, and single quotes are handled too (also missed before). Verified in both directions rather than assumed: the two new harness cases fail against the current installer (29 passed / 2 failed) and pass with the fix (31 passed / 0 failed), plus ten direct variants covering quoting styles, multi-path lines, flags, comments and negative cases. Resolving — this was the last open finding from today's staging wave. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated promotion by the release train (RFC-0008 D14). Head is the train-managed
release-train/to-stagingbranch (a mirror ofdevelop), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Installer changes edit user shell rc files with nuanced ownership rules—high user impact but heavily tested; release workflow changes affect how signed rebuilds must be triggered.
Overview
Bumps VERSION to
0.10.2and tightens CI/release plumbing alongside a large Unix installer fix for shell profile PATH handling.install.shstops stacking duplicate “tracebloc installer” PATH blocks on every re-install or prefix change (#433). It tags owned blocks with a stable marker (now including(prefix: …)), strips only blocks it can prove it wrote, then replaces or appends a single block. Ownership is conservative so dangling markers cannot remove user aliases or PATH lines (#434). PATH detection uses whole path components (not substring grep), skips rc writes when the prefix is already persisted, avoids rewriting unchanged files on upgrade, and distinguishes cosmetic tidy failures from real persist failures in user messaging.Release workflow drops the free-form
refdispatch input: manual rebuilds must run on thev*tag so cosign identities match what installers verify. A fast Ref guard job enforces that; version/tag shell steps readREF_NAMEfrom env only (R8 injection hardening).Code quality enables
soft-fail: false, addsworkflow_dispatchfor optional whole-repo scans, and passesall-filesinto the shared quality workflow.install-verify.shisolates$HOMEin sandboxes and adds broad regression tests for idempotency, legacy markers, and install messaging.Reviewed by Cursor Bugbot for commit 19f9189. Bugbot is set up for automated code reviews on this repo. Configure here.