diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index cf6b56b..f571efd 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -2,7 +2,7 @@ ## Related - + ## Type of change - [ ] Feature @@ -10,13 +10,23 @@ - [ ] Tech-debt / refactor - [ ] Docs - [ ] Security / hardening +- [ ] Breaking change ## Test plan - + + +## Screenshots / recordings + + +## Deployment notes + ## Checklist - [ ] Tests added / updated and passing locally -- [ ] `go build ./...`, `go vet`, and the Lint job's checks pass locally -- [ ] Terminal output follows [STYLE.md](../STYLE.md) — Printer tones (no hardcoded colour/emoji), "secure environment" not "workspace"; `bash scripts/check-style.sh` passes +- [ ] Docs updated if behavior or config changed - [ ] No secrets / credentials in the diff +- [ ] For security-sensitive paths: appropriate reviewer requested - [ ] Cross-repo issues use `Fixes tracebloc/#N` — a bare `repo#N` closes nothing +- [ ] If this depends on a change in another repo: shipped **expand-then-contract** (additive first, consumers adopt later), or **Breaking change** ticked above with the rollout order in *Deployment notes* — repos promote independently, so the other change may not ship with this one +- [ ] `go build ./...`, `go vet`, and the Lint job's checks pass locally +- [ ] Terminal output follows [STYLE.md](../STYLE.md) — Printer tones (no hardcoded colour/emoji), "secure environment" not "workspace"; `bash scripts/check-style.sh` passes diff --git a/.github/workflows/code-quality-caller.yml b/.github/workflows/code-quality-caller.yml index 584c557..3600fcc 100644 --- a/.github/workflows/code-quality-caller.yml +++ b/.github/workflows/code-quality-caller.yml @@ -3,6 +3,14 @@ name: Code quality on: pull_request: types: [opened, reopened, synchronize, ready_for_review] + # Manual whole-tree scan (gitleaks baseline etc.) -- runs every enabled + # job in all-files mode instead of a PR diff. + workflow_dispatch: + inputs: + all-files: + description: "Scan the whole repo, not a diff" + type: boolean + default: true # Supersede the previous run when a branch is pushed again. Measured: # workflows missing this stack ~10-minute duplicate runs per push. @@ -19,4 +27,8 @@ jobs: with: python: true # repos with Python shell: true # repos with shell scripts - # soft-fail: false # flip once the backlog is clear + # The gate is armed: findings fail the job. Backlog cleared to zero + # fleet-wide + advisory soak done (backend#1303). + soft-fail: false + all-files: ${{ inputs.all-files || false }} + gitleaks-baseline: .gitleaks-baseline.json diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index bb79a56..0a74be6 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -31,12 +31,12 @@ on: push: tags: - 'v*.*.*' - workflow_dispatch: - inputs: - ref: - description: 'Tag to build (e.g. v0.1.0). Must already exist on origin.' - required: true - type: string + # Rebuilds: dispatch the workflow AT the tag ref (Actions -> Run workflow -> + # pick the v* tag), or gh run rerun a previous tag run. Cosign embeds the + # RUN's ref in the keyless identity; the installers only trust + # @refs/tags/v.*, so a branch-dispatched "rebuild of a tag" would publish + # signatures every customer install rejects (Bugbot on promotion #428). + workflow_dispatch: {} permissions: contents: write # create / update the GitHub Release @@ -47,9 +47,27 @@ concurrency: cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: + # Pre-flight: reject a branch-misdispatch ONCE, in seconds, before the 8-way + # matrix spins up (review nit on #429). Ref passed via env, never inline. + guard: + name: Ref guard + runs-on: ubuntu-latest + env: + REF_FULL: ${{ github.ref }} + EVENT_NAME: ${{ github.event_name }} + steps: + - name: Dispatch rebuilds must run at a v* tag ref + run: | + if [ "$EVENT_NAME" = "workflow_dispatch" ] && ! printf '%s' "$REF_FULL" | grep -qE '^refs/tags/v'; then + echo "::error::rebuilds must be dispatched from the v* tag itself (Actions -> Run workflow -> select the tag), not a branch -- signatures would embed $REF_FULL and fail every customer verification." + exit 1 + fi + echo "ref ok: $REF_FULL" + release: timeout-minutes: 20 name: Build + sign + publish + needs: guard runs-on: ubuntu-latest strategy: fail-fast: false @@ -78,7 +96,7 @@ jobs: - name: Checkout uses: actions/checkout@v7 with: - ref: ${{ inputs.ref || github.ref }} + ref: ${{ github.ref }} fetch-depth: 0 - name: Set up Go @@ -94,10 +112,14 @@ jobs: - name: Determine release version id: version + env: + # Passed via env, never interpolated into the script: git permits $/ + # backticks in tag names and a crafted v* tag would otherwise execute + # on the runner (R8; same rule as the client installer workflows). + REF_NAME: ${{ github.ref_name }} run: | - # On a tag push: github.ref_name = "v0.1.0" - # On workflow_dispatch: inputs.ref = "v0.1.0" - REF="${{ inputs.ref || github.ref_name }}" + # github.ref_name = "v0.1.0" (tag push, or dispatch AT the tag ref) + REF="$REF_NAME" # Strip the leading v for use in -X main.version VERSION="${REF#v}" # The VERSION file declares the next release (read by the release @@ -186,7 +208,7 @@ jobs: - name: Checkout uses: actions/checkout@v7 with: - ref: ${{ inputs.ref || github.ref }} + ref: ${{ github.ref }} - name: Download all matrix artifacts uses: actions/download-artifact@v7 @@ -215,8 +237,15 @@ jobs: - name: Determine release tag id: tag + env: + # Pass the ref through the environment, never interpolate it into the + # script. A tag name is attacker-controllable, so a crafted v* tag + # carrying backticks or $() would otherwise execute on the publish + # runner before the release is even created (R8). Same treatment the + # guard and version steps above already got. + REF_NAME: ${{ github.ref_name }} run: | - REF="${{ inputs.ref || github.ref_name }}" + REF="$REF_NAME" # STRICT stability rule: only a plain vX.Y.Z tag is a stable release. # Anything else (v1.2.3-rc.1, and typos like v1.2.3rc1) is marked # prerelease, so it can never become 'latest' -- which is what the diff --git a/.gitleaks-baseline.json b/.gitleaks-baseline.json new file mode 100644 index 0000000..d9c7cbd --- /dev/null +++ b/.gitleaks-baseline.json @@ -0,0 +1,23 @@ +[ + { + "RuleID": "generic-api-key", + "Description": "Detected a Generic API Key, potentially exposing access to various services and sensitive operations.", + "StartLine": 212, + "EndLine": 212, + "StartColumn": 4, + "EndColumn": 45, + "Match": "IdempotencyKey: \"REDACTED\"", + "Secret": "REDACTED", + "File": "internal/submit/submit_test.go", + "SymlinkFile": "", + "Commit": "d2754bd0e71bd5363b7c7f51a286b7d705e7928d", + "Link": "https://github.com/tracebloc/cli/blob/d2754bd0e71bd5363b7c7f51a286b7d705e7928d/internal/submit/submit_test.go#L212", + "Entropy": 4.004886, + "Author": "lukasWuttke", + "Email": "54042461+LukasWodka@users.noreply.github.com", + "Date": "2026-07-13T11:27:35Z", + "Message": "feat(push): print the run's correlation id on submit (backend#1028 item 3) (#245)\n\nThe idempotency key the CLI already sends is becoming the end-to-end\ningest correlation id: jobs-manager derives the Job name from it, labels\nevery spawned resource with it, and (client-runtime) stamps it into the\ningestor container as TRACEBLOC_INGEST_CORRELATION_ID, where the\ningestor (data-ingestors) logs it and carries it into the backend\nregistration payload.\n\nThe CLI was the only layer that never showed the key, so the customer\nhad no copy of the one string that threads all layers together. Print\nit as a hint line on every submit path — fresh and replay (a replayed\nrun is exactly when you reach for the id to find the already-running\nJob).\n\nNo wire change: the key was already in the POST body.\n\nCo-authored-by: Claude Fable 5 \u003cnoreply@anthropic.com\u003e", + "Tags": [], + "Fingerprint": "d2754bd0e71bd5363b7c7f51a286b7d705e7928d:internal/submit/submit_test.go:generic-api-key:212" + } +] diff --git a/VERSION b/VERSION index 5712157..5eef0f1 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.10.1 +0.10.2 diff --git a/scripts/install.sh b/scripts/install.sh index 5886c61..12d32e7 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -489,6 +489,255 @@ on_path=no # means the binary is usable now, so don't nag "open a new terminal". case ":$PATH:" in *":$PREFIX:"*|*":$PREFIX/:"*) on_path=yes ;; esac +# -------------------------------------------------------------------- +# The ONE PATH block this installer owns. +# +# Before #433 the append was guarded only by "does the rc mention $PREFIX", so +# every install with a DIFFERENT prefix appended another block and the profile +# grew without bound: the v0.10.1 validation box collected TEN, each pointing at +# a temp dir that no longer existed. We now tag our block with $PATH_MARKER and +# a re-run REPLACES it instead of stacking a second one. +# +# Keep $PATH_MARKER byte-stable — matching it is exactly what lets a re-run find +# and clean up the blocks older installers left behind. +# -------------------------------------------------------------------- +PATH_MARKER="# Added by the tracebloc CLI installer" + +# The marker line we WRITE also records the directory, so a later run can prove +# it wrote both halves of the block: +# +# # Added by the tracebloc CLI installer (prefix: /opt/tracebloc) +# export PATH="/opt/tracebloc:$PATH" +# +# When the recorded prefix and the directory on the PATH line agree, the block is +# unambiguously ours and can be reclaimed even if that directory has since been +# deleted — which is what keeps the #433 cleanup working. $PATH_MARKER stays the +# stable *beginning* of the line (matched literally, never as a whole line) so +# blocks written by older installers are still recognised. +PATH_MARKER_LINE="$PATH_MARKER (prefix: $PREFIX)" + +# _tb_owns_dir : 0 if the block naming , under a marker that +# recorded ("-" when it recorded nothing), is one we wrote. +# +# The marker alone is NOT proof. A marker left dangling by a hand-edit can end up +# directly above the USER's own PATH line, and taking that line with it would +# silently delete a PATH entry we never added — unrecoverable, and the one outcome +# this whole change exists to avoid (Bugbot on #434). So we require positive +# evidence, in order: +# * a '$' anywhere in the dir -> never ours. We always write a literal, +# already-expanded path; "$HOME/mytools" is the user's own idiom. +# * the dir we are installing to right now -> safe to replace whoever wrote it, +# since we are about to write that very line. +# * the marker recorded this same dir -> we wrote the comment AND the line. +# Existence is irrelevant here: a recorded prefix that has been deleted is +# exactly the stale cruft #433 reported. +# * a legacy marker (recorded nothing) -> the only proof left is a tracebloc +# binary still sitting in the dir. A legacy marker over a VANISHED dir is +# deliberately NOT claimed: it is indistinguishable from the user's own entry +# for a directory they haven't created yet, so we keep their line and accept +# that one pre-#433 dead entry may survive. +_tb_owns_dir() { + _tow_rec="$1" + _tow_dir="$2" + case "$_tow_dir" in + ''|-|*'$'*) return 1 ;; + esac + if [ "$_tow_dir" = "$PREFIX" ]; then return 0; fi + if [ "$_tow_rec" != "-" ] && [ "$_tow_rec" = "$_tow_dir" ]; then return 0; fi + if [ "$_tow_rec" = "-" ] && [ -d "$_tow_dir" ] && [ -e "$_tow_dir/$BINARY_NAME" ]; then return 0; fi + return 1 +} + +# strip_tb_path_block : echo minus every block we can PROVE we wrote +# — the marker line, the PATH op on the line after it, and the blank separator on +# the line before, but only when _tb_owns_dir vouches for the directory named. +# +# A block we cannot claim is left completely intact, comment and all. Two reasons: +# we must never delete a PATH line we didn't write, and a pre-#433 block we can't +# claim is far more useful to the user still labelled "Added by the tracebloc CLI +# installer" (so they can see what it is and delete it) than reduced to a bare, +# unexplained PATH line. Every other line is passed through unchanged and in +# order: this is a file we do NOT own. +strip_tb_path_block() { + _stb_file="$1" + _stb_owned="" + _stb_tab="$(printf '\t')" + + # Emit one record per marker: its line number, the prefix that marker recorded + # ("-" for a legacy marker that recorded none), and the directory named on the + # line below it ("-" when that line isn't a PATH op shaped like one we write). + # Via a temp file rather than a pipe so the ownership verdicts land in THIS + # shell, and rather than a heredoc so the awk program needs no + # nested-expansion escaping. + # + # A marker is any line STARTING with $PATH_MARKER (index(...) == 1 is a + # literal test, so a path full of regex metacharacters can't misfire). + _stb_pairs="$TMP/tb.pairs" + awk -v marker="$PATH_MARKER" ' + function recorded_prefix(line, rest) { + rest = substr(line, length(marker) + 1) + # " (prefix: )" -> ; anything else (including the bare + # legacy marker) records nothing. + if (rest ~ /^ \(prefix: .*\)$/) return substr(rest, 11, length(rest) - 11) + return "-" + } + NR > 1 && prev_is_marker { + dir = "-" + if ($0 ~ /^[[:space:]]*export[[:space:]]+PATH="[^"]+:\$PATH"[[:space:]]*$/) { + dir = $0 + sub(/^[^"]*"/, "", dir) + sub(/:\$PATH".*$/, "", dir) + } else if ($0 ~ /^[[:space:]]*fish_add_path[[:space:]]/) { + dir = $0 + sub(/^[[:space:]]*fish_add_path[[:space:]]+/, "", dir) + sub(/[[:space:]]+$/, "", dir) + gsub(/^["\047]|["\047]$/, "", dir) + } + printf "%d\t%s\t%s\n", NR - 1, prev_rec, dir + } + { + prev_is_marker = (index($0, marker) == 1) + prev_rec = prev_is_marker ? recorded_prefix($0) : "-" + } + END { if (NR >= 1 && prev_is_marker) printf "%d\t%s\t-\n", NR, prev_rec } + ' "$_stb_file" > "$_stb_pairs" + + while IFS="$_stb_tab" read -r _stb_no _stb_rec _stb_dir; do + [ -n "$_stb_no" ] || continue + if _tb_owns_dir "$_stb_rec" "$_stb_dir"; then + _stb_owned="$_stb_owned,$_stb_no" + fi + done < "$_stb_pairs" + + awk -v owned="$_stb_owned" ' + BEGIN { + n = split(owned, a, ","); for (i = 1; i <= n; i++) if (a[i] != "") own[a[i] + 0] = 1 + } + { line[NR] = $0 } + END { + for (i = 1; i <= NR; i++) { + if (!(i in own)) continue + drop[i] = 1 # the marker + drop[i + 1] = 1 # the PATH op under it (vouched for) + if (i > 1 && line[i - 1] == "") drop[i - 1] = 1 # our blank separator + } + for (i = 1; i <= NR; i++) if (!(i in drop)) print line[i] + } + ' "$_stb_file" +} + +# rc_lists_dir : 0 if a non-comment PATH op in already puts +# on PATH. Only an actual PATH op counts — a bare comment or an unrelated +# line that merely mentions the dir must NOT pass, or we'd claim success while a +# new shell still can't find the binary (#61). Handles PATH= / PATH+= / zsh's +# path+=() / fish_add_path. +# +# It compares whole path COMPONENTS. The old test was `grep -F "$PREFIX"`, a +# substring match: installing --prefix /opt/tb after /opt/tb2 matched the +# /opt/tb2 line and reported "already in your PATH config" for a directory that +# was on nobody's PATH (#433). Quotes and trailing slashes are tolerated on +# either side. +rc_lists_dir() { + awk -v want="$1" ' + # exit 1, not a bare exit. A bare exit in BEGIN still runs END, so + # the status would be the END rule below -- 1 when nothing was found -- + # but saying it outright means "no component to look for" can never be + # read as "already listed" by a later reader or a stricter awk. + BEGIN { sub(/\/+$/, "", want); if (want == "") exit 1 } + /^[[:space:]]*#/ { next } + { + n = 0 + if ($0 ~ /(^|[^A-Za-z_])fish_add_path([^A-Za-z_]|$)/) { + # Parse quotes instead of field-splitting. We WRITE + # fish_add_path "$PREFIX", so a prefix containing spaces became + # two whitespace fields here and never matched -- the installer + # then appended a second block and claimed it had added a PATH + # entry that was already present. The sibling reader further up + # already keeps the remainder intact; this now agrees with it. + # Tokenise the argument list, honouring quotes. Three things + # this has to get right, each of which broke a previous + # attempt (cli#439): + # * FIRST occurrence of the command, via index() -- a greedy + # /^.*fish_add_path/ strips through an inline comment that + # mentions it and loses the real argument; + # * quoted arguments are ONE path, spaces included, because we + # write fish_add_path "$PREFIX"; + # * fish_add_path takes MULTIPLE directories, so parsing must + # continue past the first closing quote. + at = index($0, "fish_add_path") + rest = substr($0, at + 13) + while (length(rest) > 0) { + sub(/^[[:space:]]+/, "", rest) + if (length(rest) == 0) break + ch = substr(rest, 1, 1) + if (ch == "#") break # trailing comment + if (ch == "\"" || ch == "\047") { + body = substr(rest, 2) + close_at = index(body, ch) + if (close_at > 0) { + cand[++n] = substr(body, 1, close_at - 1) + rest = substr(body, close_at + 1) + } else { # unterminated quote + cand[++n] = body + rest = "" + } + } else { + sp = match(rest, /[[:space:]]/) + tok = (sp > 0) ? substr(rest, 1, sp - 1) : rest + rest = (sp > 0) ? substr(rest, sp) : "" + if (tok !~ /^-/) cand[++n] = tok # skip flags + } + } + } else if ($0 ~ /(^|[^A-Za-z_])[Pp][Aa][Tt][Hh][+]?=/) { + value = $0 + sub(/^[^=]*=/, "", value) + n = split(value, cand, ":") + } else { + next + } + for (i = 1; i <= n; i++) { + dir = cand[i] + gsub(/[\047"()]/, "", dir) + sub(/\/+$/, "", dir) + if (dir == want) { found = 1; exit } + } + } + END { exit(found ? 0 : 1) } + ' "$2" +} + +# rc_same : 0 if the two files hold the same text. Used to skip +# the write entirely when the rc already says what we were going to say — +# `tracebloc upgrade` re-execs this installer, so the same prefix comes back +# around on every upgrade and a file we don't own must not be rewritten to no +# effect. +# +# Deliberately not cmp(1): cmp ships in diffutils, which a minimal container +# image can lack, and a missing tool would silently turn "leave the file alone" +# into "rewrite it every run". $(...) strips trailing newlines on both sides +# equally, which is harmless here — a missing block is a far bigger difference +# than a final newline. +rc_same() { + [ "$(cat "$1")" = "$(cat "$2")" ] +} + +# replace_rc : make $rc's contents exactly 's. Truncates the +# existing path rather than mv'ing a temp over it, so the inode, mode and owner +# survive — and so an rc that is a SYMLINK into a dotfiles repo is written +# THROUGH instead of being silently replaced by a regular file. +replace_rc() { + if { cat "$1" > "$rc"; } 2>/dev/null; then + return 0 + fi + # The redirection truncates before cat writes, so a write that dies partway + # (no space left, a vanishing mount) would leave the user's rc in pieces. + # Put the original contents back — best effort, but far better than leaving a + # file we don't own half-written. The caller then reports `failed` and prints + # the line to add by hand. + { cat "$rc_now" > "$rc"; } 2>/dev/null || true + return 1 +} + if [ "$persist" = "yes" ]; then shell_name="$(basename "${SHELL:-sh}")" case "$shell_name" in @@ -503,32 +752,74 @@ if [ "$persist" = "yes" ]; then esac if [ "$shell_name" = "fish" ]; then - path_line="fish_add_path $PREFIX" + # Quoted, like the client installer's fish hint: a --prefix containing a + # space must survive into the rc as one argument. + path_line="fish_add_path \"$PREFIX\"" else path_line="export PATH=\"$PREFIX:\$PATH\"" fi - # Track three outcomes precisely so the message can neither over- nor - # under-claim: already configured / freshly added / couldn't write. - state=failed + # fish's rc lives in ~/.config/fish/, which may not exist yet; the others sit + # directly in $HOME. Create the parent or the write below has nowhere to go. mkdir -p "$(dirname "$rc")" 2>/dev/null || true - # Idempotency: only an actual, non-comment PATH op that references $PREFIX - # counts as "already configured" — a bare comment or an unrelated line that - # merely mentions the dir must NOT pass, or we'd claim success while a new - # shell still can't find the binary (#61). Match PATH= / PATH+= / - # fish_add_path / zsh's path+=() (case-insensitive); the [^A-Za-z_] guard - # keeps PYTHONPATH=/MYPATH= out. - if grep -v '^[[:space:]]*#' "$rc" 2>/dev/null \ - | grep -iE '(^|[^A-Za-z_])path[+]?=|fish_add_path' \ - | grep -qF "$PREFIX"; then - state=present # rc already persists it — leave it alone - # Group the append so the redirection-open error (e.g. a read-only rc, or - # an unwritable parent dir) is suppressed too: `cmd >> "$rc" 2>/dev/null` - # leaks the shell's "Permission denied" because the >> open is attempted - # before 2>/dev/null applies. Wrapping in { ... } 2>/dev/null puts the - # stderr redirect in scope first. - elif { printf '\n# Added by the tracebloc CLI installer\n%s\n' "$path_line" >> "$rc"; } 2>/dev/null; then - state=added + + # Read the rc as it stands (it may not exist yet) and compute what it looks + # like with our block removed. Deciding from the STRIPPED copy is what makes + # this idempotent: whatever we wrote on an earlier run is out of the way, so + # "is $PREFIX already handled?" is answered by the user's own lines only, and + # our block gets rewritten rather than duplicated. + rc_now="$TMP/rc.current" + rc_next="$TMP/rc.next" + rc_want="$TMP/rc.want" + : > "$rc_now" + if [ -f "$rc" ]; then cat "$rc" > "$rc_now" 2>/dev/null || : > "$rc_now"; fi + strip_tb_path_block "$rc_now" > "$rc_next" + + # Build the contents the rc SHOULD have. If a line we don't own already puts + # $PREFIX on PATH we add nothing — the stripped copy is already the answer, + # and any block of ours it removed was redundant. + cat "$rc_next" > "$rc_want" + if rc_lists_dir "$PREFIX" "$rc_next"; then + persisted=yes + else + persisted=no + printf '\n%s\n%s\n' "$PATH_MARKER_LINE" "$path_line" >> "$rc_want" + fi + + # Six outcomes, tracked precisely so the closing message can neither over- nor + # under-claim. The two failure states are distinct on purpose: only one of them + # means the user's PATH is actually wrong (Bugbot on #434). + # present — the rc already says this; nothing written + # added — our block appended to an rc that had none of ours + # replaced — our stale block rewritten to name $PREFIX + # tidied — $PREFIX was already persisted by the user; we removed a + # redundant block of ours + # tidy_failed — as `tidied`, but the rewrite failed. PATH is still correct, + # so this is cosmetic and must NOT ask for a manual edit. + # failed — $PREFIX is not persisted and we could not write. The only + # case that warrants manual instructions. + state=failed + if rc_same "$rc_want" "$rc_now"; then + # Never touch a file we don't own to no effect — the re-install and + # `tracebloc upgrade` path lands here. + state=present + elif rc_same "$rc_next" "$rc_now"; then + # Nothing of ours was stripped, so the only difference is our new block: + # append rather than rewriting the whole file. (Reachable only when + # $persisted is no — otherwise rc_want would equal rc_now above.) + # + # Group the append so the redirection-open error (e.g. a read-only rc, or + # an unwritable parent dir) is suppressed too: `cmd >> "$rc" 2>/dev/null` + # leaks the shell's "Permission denied" because the >> open is attempted + # before 2>/dev/null applies. Wrapping in { ... } 2>/dev/null puts the + # stderr redirect in scope first. + if { printf '\n%s\n%s\n' "$PATH_MARKER_LINE" "$path_line" >> "$rc"; } 2>/dev/null; then + state=added + fi + elif replace_rc "$rc_want"; then + if [ "$persisted" = yes ]; then state=tidied; else state=replaced; fi + elif [ "$persisted" = yes ]; then + state=tidy_failed fi echo "" @@ -544,13 +835,40 @@ if [ "$persist" = "yes" ]; then echo "Open a new terminal — or load it now: . \"$rc\"" fi ;; - present) + replaced) + # An earlier install left a PATH line for a different prefix. We + # updated that one line in place instead of stacking another (#433). + echo "Updated the tracebloc PATH entry in $rc to $PREFIX." + if [ "$on_path" = yes ]; then + echo "tracebloc is ready to use now." + else + echo "Open a new terminal — or load it now: . \"$rc\"" + fi + ;; + present|tidied) + # `tidied` differs only in that we also dropped a redundant block of + # ours; either way the rc already persists $PREFIX, so the user has + # nothing to do. + if [ "$on_path" = yes ]; then + echo "tracebloc is ready to use now ($PREFIX is on your PATH)." + else + echo "$PREFIX is already in your PATH config ($rc) — nothing to add." + echo "If a new terminal can't find it yet, open one — or load it now: . \"$rc\"" + fi + ;; + tidy_failed) + # $PREFIX IS persisted — by a line we don't own — and all we failed to + # do is remove a now-redundant block of ours. Telling the user to add a + # PATH line by hand here would be plain wrong: their PATH is correct. + # Say what actually happened and leave it at that (Bugbot on #434). if [ "$on_path" = yes ]; then echo "tracebloc is ready to use now ($PREFIX is on your PATH)." else echo "$PREFIX is already in your PATH config ($rc) — nothing to add." echo "If a new terminal can't find it yet, open one — or load it now: . \"$rc\"" fi + echo "Note: couldn't remove a leftover tracebloc PATH line from $rc" + echo " (not writable). Harmless — your PATH is already correct." ;; *) # Usable in THIS shell already ($PREFIX on PATH) — say so even though diff --git a/scripts/tests/install-verify.sh b/scripts/tests/install-verify.sh index f499394..58d30a1 100755 --- a/scripts/tests/install-verify.sh +++ b/scripts/tests/install-verify.sh @@ -32,8 +32,13 @@ _sha() { if command -v sha256sum >/dev/null 2>&1; then sha256sum "$1" | awk '{pr # Build a sandbox release + mock bin. Args: COSIGN_PRESENT(yes/no) make_sandbox() { SBX="$(mktemp -d)" - BIN="$SBX/bin"; REL="$SBX/release"; DEST="$SBX/dest" - mkdir -p "$BIN" "$REL" "$DEST" + BIN="$SBX/bin"; REL="$SBX/release"; DEST="$SBX/dest"; HOMEDIR="$SBX/home" + # Every run gets its OWN $HOME. The installer persists a PATH line to the + # shell rc, so without this the harness appends to the *developer's* real + # ~/.bash_profile — once per successful case, with a fresh mktemp --prefix + # each time. That is how the v0.10.1 validation box ended up with ten + # tracebloc PATH blocks, all naming temp dirs that no longer existed (#433). + mkdir -p "$BIN" "$REL" "$DEST" "$HOMEDIR" # The "binary" and its SHA256SUMS, named exactly as resolve_tag/detect_* expect. os="$(uname -s | tr '[:upper:]' '[:lower:]')"; [ "$os" = darwin ] || os=linux @@ -77,8 +82,31 @@ EOF drop_sandbox() { rm -rf "$SBX"; } -# Run installer with PATH=$BIN only (host cosign can't shadow), into $DEST. -run_installer() { PATH="$BIN" "$BIN/bash" "$INSTALLER" --prefix "$DEST" "$@" >"$SBX/out" 2>&1; echo $?; } +# Run installer with PATH=$BIN only (host cosign can't shadow), into $DEST, with +# $HOME pointed at the sandbox so the rc write can never touch real dotfiles. +# FAKE_SHELL selects which rc the installer targets; /bin/sh → $HOME/.profile, +# which is the same answer on Linux and macOS (bash differs between them), so the +# assertions below don't have to branch per platform. +run_installer() { + PATH="$BIN" HOME="$HOMEDIR" SHELL="${FAKE_SHELL:-/bin/sh}" \ + "$BIN/bash" "$INSTALLER" --prefix "$DEST" "$@" >"$SBX/out" 2>&1 + echo $? +} + +# Same, but installs into an explicit prefix (and optionally with that prefix +# pre-seeded onto PATH) so the profile-write behaviour can be exercised across +# several installs that share one $HOME. +run_installer_at() { + local prefix="$1" extra_path="${2:-}" + PATH="$BIN${extra_path:+:$extra_path}" HOME="$HOMEDIR" SHELL="${FAKE_SHELL:-/bin/sh}" \ + "$BIN/bash" "$INSTALLER" --prefix "$prefix" >"$SBX/out" 2>&1 + echo $? +} + +# How many PATH blocks the installer owns in the sandbox rc. Prefix match, not a +# whole-line one: the marker we write now carries a " (prefix: )" suffix, +# while blocks from older installers are the bare comment. +tb_blocks() { grep -c '^# Added by the tracebloc CLI installer' "$1" 2>/dev/null || true; } # ── 1. cosign present + valid signature → installs ────────────────────────── make_sandbox yes @@ -209,6 +237,298 @@ else fi drop_sandbox +# ── 11. the rc PATH block is idempotent and never accumulates (#433) ───────── +# Pre-#433 the append was guarded only by "does the rc mention $PREFIX", so an +# install with a DIFFERENT prefix appended another block every time and the +# profile grew without bound. Assert exactly ONE block whatever the history: +# same prefix repeated, then a run of distinct prefixes. +make_sandbox yes +RC="$HOMEDIR/.profile" +COSIGN_RESULT=0 run_installer >/dev/null +COSIGN_RESULT=0 run_installer >/dev/null +COSIGN_RESULT=0 run_installer >/dev/null +if [ "$(tb_blocks "$RC")" = 1 ]; then + ok "same prefix installed 3x leaves one PATH block" +else + bad "same prefix installed 3x leaves one PATH block (got $(tb_blocks "$RC"))"; sed 's/^/ /' "$RC" +fi + +# …and the repeat runs must not even rewrite the file. `tracebloc upgrade` +# re-execs this installer, so the same prefix comes back on every upgrade; the rc +# is the user's file and an install that changes nothing must touch nothing. +cp "$RC" "$SBX/rc.before" +COSIGN_RESULT=0 run_installer >/dev/null +if diff -q "$RC" "$SBX/rc.before" >/dev/null && grep -q 'already in your PATH config' "$SBX/out"; then + ok "re-install with the same prefix leaves the rc byte-identical" +else + bad "re-install with the same prefix leaves the rc byte-identical"; diff "$SBX/rc.before" "$RC" | sed 's/^/ /' +fi + +# Distinct prefixes — the exact shape that produced ten blocks on the v0.10.1 box. +p1="$SBX/p1"; p2="$SBX/p2"; p3="$SBX/p3" +for p in "$p1" "$p2" "$p3"; do COSIGN_RESULT=0 run_installer_at "$p" >/dev/null; done +if [ "$(tb_blocks "$RC")" = 1 ] && grep -qF "$p3" "$RC" && ! grep -qF "$p1" "$RC"; then + ok "three different prefixes leave one block, naming the newest" +else + bad "three different prefixes leave one block, naming the newest (got $(tb_blocks "$RC"))"; sed 's/^/ /' "$RC" +fi +drop_sandbox + +# ── 12. a prefix already on PATH is not persisted at all ──────────────────── +# Nothing to fix, so the rc must not be created or touched — the common +# /usr/local/bin case used to get a line that changed nothing (#433). +make_sandbox yes +COSIGN_RESULT=0 run_installer_at "$DEST" "$DEST" >/dev/null +if [ ! -e "$HOMEDIR/.profile" ]; then + ok "prefix already on PATH writes no rc at all" +else + bad "prefix already on PATH writes no rc at all"; sed 's/^/ /' "$HOMEDIR/.profile" +fi +drop_sandbox + +# ── 13. the user's own rc content survives byte-for-byte ──────────────────── +# We are editing a file we don't own: replacing our block must not reorder, +# rewrite or drop a single unrelated line. +make_sandbox yes +RC="$HOMEDIR/.profile" +cat > "$RC" <<'PROF' +# my profile +export EDITOR=vim +export PATH="$HOME/mytools:$PATH" + +alias ll='ls -la' +PROF +cp "$RC" "$SBX/rc.orig" +COSIGN_RESULT=0 run_installer_at "$SBX/q1" >/dev/null +COSIGN_RESULT=0 run_installer_at "$SBX/q2" >/dev/null +# Strip our block back out; what remains must equal the original exactly. +grep -vF '# Added by the tracebloc CLI installer' "$RC" | grep -vF "$SBX/q2" | sed '${/^$/d;}' > "$SBX/rc.stripped" +if [ "$(tb_blocks "$RC")" = 1 ] && diff -q "$SBX/rc.stripped" "$SBX/rc.orig" >/dev/null; then + ok "unrelated rc lines preserved across a block replacement" +else + bad "unrelated rc lines preserved across a block replacement"; diff "$SBX/rc.orig" "$SBX/rc.stripped" | sed 's/^/ /' +fi +drop_sandbox + +# ── 14. a marker left dangling above unrelated content can't eat it ───────── +# Removal keys off our marker, so it only takes the next line when that line is +# shaped like a PATH op we wrote — never arbitrary user content. +make_sandbox yes +RC="$HOMEDIR/.profile" +printf '# Added by the tracebloc CLI installer\nalias precious="keep me"\n' > "$RC" +COSIGN_RESULT=0 run_installer_at "$SBX/r1" >/dev/null +if grep -q 'precious' "$RC"; then + ok "dangling marker doesn't consume the line below it" +else + bad "dangling marker doesn't consume the line below it"; sed 's/^/ /' "$RC" +fi +drop_sandbox + +# The nastier variant: a dangling marker directly above the user's OWN PATH +# export. The marker is not proof of ownership, so the line only goes if we can +# vouch for the directory it names — otherwise we'd silently delete a PATH entry +# we never added (Bugbot on #434). +make_sandbox yes +RC="$HOMEDIR/.profile" +mkdir -p "$SBX/mytools" +printf '# Added by the tracebloc CLI installer\nexport PATH="%s:$PATH"\n' "$SBX/mytools" > "$RC" +COSIGN_RESULT=0 run_installer_at "$SBX/t1" >/dev/null +if grep -qF "$SBX/mytools" "$RC"; then + ok "dangling marker above the user's own PATH export keeps that export" +else + bad "dangling marker above the user's own PATH export keeps that export"; sed 's/^/ /' "$RC" +fi +# Same, with an unexpanded $HOME — a '$' can never appear in a path we wrote. +printf '# Added by the tracebloc CLI installer\nexport PATH="$HOME/mytools:$PATH"\n' > "$RC" +COSIGN_RESULT=0 run_installer_at "$SBX/t2" >/dev/null +if grep -qF '$HOME/mytools' "$RC"; then + ok "dangling marker above an unexpanded \$HOME PATH line keeps that line" +else + bad "dangling marker above an unexpanded \$HOME PATH line keeps that line"; sed 's/^/ /' "$RC" +fi +drop_sandbox + +# The nastiest variant of all, and why the marker now records its prefix: a +# dangling marker above the user's own literal-path line for a directory they have +# NOT created yet. "The directory vanished" used to be taken as proof the block was +# ours, which deleted that line (Bugbot on #434). A legacy marker — one that +# recorded no prefix — can no longer claim a missing directory. +make_sandbox yes +RC="$HOMEDIR/.profile" +printf '# Added by the tracebloc CLI installer\nexport PATH="%s/not-created-yet:$PATH"\n' "$SBX" > "$RC" +COSIGN_RESULT=0 run_installer_at "$SBX/t3" >/dev/null +if grep -qF 'not-created-yet' "$RC"; then + ok "legacy marker can't claim a missing dir — user's line survives" +else + bad "legacy marker can't claim a missing dir — user's line survives"; sed 's/^/ /' "$RC" +fi +# Nor an existing directory that holds no tracebloc binary. +printf '# Added by the tracebloc CLI installer\nexport PATH="%s/mytools:$PATH"\n' "$SBX" > "$RC" +COSIGN_RESULT=0 run_installer_at "$SBX/t4" >/dev/null +if grep -qF '/mytools:' "$RC"; then + ok "legacy marker can't claim a dir without our binary" +else + bad "legacy marker can't claim a dir without our binary"; sed 's/^/ /' "$RC" +fi +drop_sandbox + +# …but a block whose marker RECORDS the prefix is provably ours, so a vanished +# directory is still reclaimed — that is the #433 cruft, and cleaning it is the +# whole point. This is what a post-fix installer writes. +make_sandbox yes +RC="$HOMEDIR/.profile" +printf '\n# Added by the tracebloc CLI installer (prefix: %s/gone)\nexport PATH="%s/gone:$PATH"\n' "$SBX" "$SBX" > "$RC" +COSIGN_RESULT=0 run_installer_at "$SBX/t5" >/dev/null +if [ "$(tb_blocks "$RC")" = 1 ] && ! grep -qF '/gone:' "$RC"; then + ok "a recorded-prefix block naming a vanished dir is cleaned up" +else + bad "a recorded-prefix block naming a vanished dir is cleaned up"; sed 's/^/ /' "$RC" +fi +# Ten of them — the shape the v0.10.1 box was in — collapse to one. +: > "$RC" +i=1 +while [ "$i" -le 10 ]; do + printf '\n# Added by the tracebloc CLI installer (prefix: /tmp/dead-%s)\nexport PATH="/tmp/dead-%s:$PATH"\n' "$i" "$i" >> "$RC" + i=$((i+1)) +done +COSIGN_RESULT=0 run_installer_at "$SBX/t6" >/dev/null +if [ "$(tb_blocks "$RC")" = 1 ] && ! grep -qF '/tmp/dead-' "$RC"; then + ok "ten recorded-prefix blocks collapse to one" +else + bad "ten recorded-prefix blocks collapse to one (got $(tb_blocks "$RC"))"; sed 's/^/ /' "$RC" +fi +drop_sandbox + +# An unclaimable legacy block must not cause write CHURN either: we leave it +# alone, add ours once, and every later run with the same prefix is a no-op. +make_sandbox yes +RC="$HOMEDIR/.profile" +mkdir -p "$SBX/legacy" +printf '# Added by the tracebloc CLI installer\nexport PATH="%s/legacy:$PATH"\n' "$SBX" > "$RC" +COSIGN_RESULT=0 run_installer_at "$SBX/t7" >/dev/null +cp "$RC" "$SBX/rc.after1" +COSIGN_RESULT=0 run_installer_at "$SBX/t7" >/dev/null +COSIGN_RESULT=0 run_installer_at "$SBX/t7" >/dev/null +if diff -q "$RC" "$SBX/rc.after1" >/dev/null && grep -qF '/legacy:' "$RC"; then + ok "an unclaimable legacy block is kept without churning the rc" +else + bad "an unclaimable legacy block is kept without churning the rc"; diff "$SBX/rc.after1" "$RC" | sed 's/^/ /' +fi +drop_sandbox + +# Finding 2: when the user's OWN line already persists the prefix and all we fail +# to do is remove a redundant block of ours, the rc is already correct — so the +# installer must NOT tell them to add a PATH line by hand. +make_sandbox yes +RC="$HOMEDIR/.profile" +printf 'export PATH="%s:$PATH"\n\n# Added by the tracebloc CLI installer (prefix: %s)\nexport PATH="%s:$PATH"\n' \ + "$DEST" "$DEST" "$DEST" > "$RC" +chmod 444 "$RC" +COSIGN_RESULT=0 run_installer >/dev/null +chmod 644 "$RC" +if ! grep -q 'Add this line to it' "$SBX/out" \ + && ! grep -q "couldn't update your shell config" "$SBX/out" \ + && grep -q 'already in your PATH config' "$SBX/out" \ + && grep -q "couldn't remove a leftover tracebloc PATH line" "$SBX/out"; then + ok "failed tidy-up of a redundant block never asks for a manual PATH line" +else + bad "failed tidy-up of a redundant block never asks for a manual PATH line"; sed 's/^/ /' "$SBX/out" +fi +drop_sandbox + +# …while a genuine failure — prefix NOT persisted anywhere, rc unwritable — must +# still give the manual instruction, because the user's PATH really is wrong. +make_sandbox yes +RC="$HOMEDIR/.profile" +printf '# nothing of ours here\n' > "$RC" +chmod 444 "$RC" +COSIGN_RESULT=0 run_installer >/dev/null +chmod 644 "$RC" +if grep -q "couldn't update your shell config" "$SBX/out" && grep -q 'Add this line to it' "$SBX/out"; then + ok "a real failure to persist still prints the manual instruction" +else + bad "a real failure to persist still prints the manual instruction"; sed 's/^/ /' "$SBX/out" +fi +drop_sandbox + +# ── 15. each shell's rc is the one a fresh interactive shell reads ────────── +# zsh → ~/.zshrc, fish → ~/.config/fish/config.fish (and fish gets +# fish_add_path, not a bash `export`), anything else → ~/.profile. Dedupe has to +# hold on whichever file we picked, so re-run and re-assert per shell. +for spec in "zsh:.zshrc:export PATH=" "fish:.config/fish/config.fish:fish_add_path"; do + sh_name="${spec%%:*}"; rest="${spec#*:}"; rc_rel="${rest%%:*}"; want_op="${rest#*:}" + make_sandbox yes + RC="$HOMEDIR/$rc_rel" + FAKE_SHELL="/bin/$sh_name" COSIGN_RESULT=0 run_installer_at "$SBX/s1" >/dev/null + FAKE_SHELL="/bin/$sh_name" COSIGN_RESULT=0 run_installer_at "$SBX/s2" >/dev/null + if [ "$(tb_blocks "$RC")" = 1 ] && grep -qF "$want_op" "$RC" && grep -qF "$SBX/s2" "$RC"; then + ok "$sh_name: one block in $rc_rel using $want_op" + else + bad "$sh_name: one block in $rc_rel using $want_op (got $(tb_blocks "$RC"))"; sed 's/^/ /' "$RC" 2>/dev/null + fi + drop_sandbox +done + +# -- 16. a fish prefix containing spaces is recognised as already listed ------ +# We WRITE `fish_add_path "$PREFIX"`, so rc_lists_dir must parse the quotes +# rather than field-split: it used to see two whitespace fields, match neither, +# append a second block, and report adding an entry that was already present. +for quote_style in double single; do + make_sandbox yes + RC="$HOMEDIR/.config/fish/config.fish" + mkdir -p "$(dirname "$RC")" + SPACED="$SBX/my tools/bin" + mkdir -p "$SPACED" + if [ "$quote_style" = double ]; then + printf 'fish_add_path "%s"\n' "$SPACED" > "$RC" + else + printf "fish_add_path '%s'\n" "$SPACED" > "$RC" + fi + before=$(cat "$RC") + FAKE_SHELL=/bin/fish COSIGN_RESULT=0 run_installer_at "$SPACED" >/dev/null 2>&1 + if [ "$(cat "$RC")" = "$before" ]; then + ok "fish: $quote_style-quoted prefix with spaces seen as already listed" + else + bad "fish: $quote_style-quoted prefix with spaces not matched (rc rewritten)"; sed 's/^/ /' "$RC" + fi + drop_sandbox +done + +# -- 17. an inline comment that mentions fish_add_path must not defeat the match +# A greedy strip through the LAST `fish_add_path` on the line would leave the +# comment text as the "path" and miss the real argument (Bugbot, cli#439). +make_sandbox yes +RC="$HOMEDIR/.config/fish/config.fish" +mkdir -p "$(dirname "$RC")" +printf 'fish_add_path "%s" # set by fish_add_path\n' "$SBX/s1" > "$RC" +mkdir -p "$SBX/s1" +before=$(cat "$RC") +FAKE_SHELL=/bin/fish COSIGN_RESULT=0 run_installer_at "$SBX/s1" >/dev/null 2>&1 +if [ "$(cat "$RC")" = "$before" ]; then + ok "fish: inline comment naming fish_add_path does not break the match" +else + bad "fish: inline comment naming fish_add_path broke the match (rc rewritten)"; sed 's/^/ /' "$RC" +fi +drop_sandbox + +# -- 18. the prefix is the SECOND quoted arg on a multi-path fish line -------- +# fish_add_path accepts several directories. Parsing that stopped at the first +# closing quote missed a later one, appended a redundant block, and reported a +# PATH add that was already there (Bugbot, cli#439). +make_sandbox yes +RC="$HOMEDIR/.config/fish/config.fish" +mkdir -p "$(dirname "$RC")" +mkdir -p "$SBX/other" "$SBX/s1" +printf 'fish_add_path "%s" "%s"\n' "$SBX/other" "$SBX/s1" > "$RC" +before=$(cat "$RC") +FAKE_SHELL=/bin/fish COSIGN_RESULT=0 run_installer_at "$SBX/s1" >/dev/null 2>&1 +if [ "$(cat "$RC")" = "$before" ]; then + ok "fish: prefix as the second quoted arg is seen as already listed" +else + bad "fish: second quoted arg was not matched (rc rewritten)"; sed 's/^/ /' "$RC" +fi +drop_sandbox + echo echo "install-verify: $PASS passed, $FAIL failed" [ "$FAIL" -eq 0 ]