release-train: staging -> main - #441
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>
release-train: develop -> staging
* fix(install): match a quoted fish path, spaces included rc_lists_dir field-split fish_add_path arguments on whitespace. Since #434 started writing fish_add_path "$PREFIX", a prefix containing spaces became two fields, matched neither, and the installer appended a SECOND block directly beneath an existing line naming the same directory -- while reporting that it had added a PATH entry that was already there. The read path now parses quotes like the ownership reader further up the file already does: a quoted argument is one path, spaces included, while an unquoted line still splits so several bare paths on one line keep working. Single quotes are handled too, which the old code also missed. Verified in both directions: the two new harness cases fail against the current installer (29 passed / 2 failed) and pass with this change (31 passed / 0 failed). Found by Bugbot on the develop->staging promotion (cli#438). * fix(install): take the FIRST fish_add_path, not the last A greedy /^.*fish_add_path/ strips through the LAST occurrence on the line, so an inline comment mentioning fish_add_path left the comment text as the path and missed the real argument -- reintroducing the exact bug this branch fixes. index() takes the first occurrence instead. Also restores the leading-whitespace strip that the greedy regex used to consume: without it substr() left ' "/path"', so the quote check saw a space and fell back to field-splitting, breaking the spaced-path fix. Caught by re-running the whole case set rather than only the new one. Bugbot, cli#439. * fix(install): take the FIRST fish_add_path, not the last A greedy /^.*fish_add_path/ strips through the LAST occurrence on the line, so an inline comment mentioning fish_add_path left the comment text as the path and missed the real argument -- reintroducing the exact bug this branch fixes. index() takes the first occurrence instead. Also restores the leading-whitespace strip that the greedy regex used to consume: without it substr() left ' "/path"', so the quote check saw a space and fell back to field-splitting, breaking the spaced-path fix. Caught by re-running the whole case set rather than only the new one. Bugbot, cli#439. * fix(install): tokenise the fish argument list properly Replaces three rounds of patching with one quote-aware tokenizer, because each patch fixed its own case and broke or missed another: * greedy .* strip lost the real argument to an inline comment * index() alone dropped the leading space, so the quote check failed and spaced paths regressed * taking only the first quoted argument missed the prefix when fish_add_path lists several directories The loop now walks the argument list: quoted tokens are one path (spaces included), bare tokens split on whitespace, flags are skipped, and a trailing # comment ends parsing. 18 direct variants and 33 harness cases green. Bugbot, cli#439. * fix(install): tokenise the fish argument list properly Replaces three rounds of patching with one quote-aware tokenizer, because each patch fixed its own case and broke or missed another: * greedy .* strip lost the real argument to an inline comment * index() alone dropped the leading space, so the quote check failed and spaced paths regressed * taking only the first quoted argument missed the prefix when fish_add_path lists several directories The loop now walks the argument list: quoted tokens are one path (spaces included), bare tokens split on whitespace, flags are skipped, and a trailing # comment ends parsing. 18 direct variants and 33 harness cases green. Bugbot, cli#439.
Records a redacted gitleaks baseline at the repo root so the full-history dispatch scan runs clean, and wires the code-quality caller to consume it via the gitleaks-baseline input. Part of tracebloc/backend#1303. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
tracebloc-release-train
Bot
added
gate-nudge
release-train: develop -> staging
Contributor
Author
This finding is already fixed on
|
Contributor
Author
|
bugbot run |
tracebloc-release-train
Bot
added
gate-nudge
Contributor
Author
Correction to my earlier comment — a
|
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 61323b3. Configure here.
tracebloc-release-train
Bot
added
gate-nudge
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 61323b3. Configure here.
tracebloc-release-train
Bot
added
gate-nudge
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-mainbranch (a mirror ofstaging), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Installer edits user shell rc files on every
curl | shinstall (high blast radius if wrong), offset by extensive harness tests; release and secret-scan workflow changes affect publishing and CI gates.Overview
Promotes 0.10.2 and ships the largest functional change in
install.sh: shell rc PATH handling is rewritten so repeated installs or prefix changes replace a single tagged block instead of stacking entries (#433), with conservative ownership checks so dangling markers cannot delete user PATH lines (#434), clearer outcomes when tidy-up fails, and fishfish_add_pathparsing fixes (spaces, multi-arg, inline comments — cli#439).Release workflow drops the free-form dispatch tag input: rebuilds must run at the
v*tag ref (new Ref guard job), and tag names are passed via env in shell steps to avoid injection (R8). Code quality turnssoft-fail: false, adds.gitleaks-baseline.json, and supports manual whole-repo scans viaworkflow_dispatch.Also updates the PR template (breaking change, deployment notes, cross-repo checklist) and extends
install-verify.shwith sandbox$HOMEand broad PATH regression tests.Reviewed by Cursor Bugbot for commit 61323b3. Bugbot is set up for automated code reviews on this repo. Configure here.