From 23c12fd50cddb615c7379e60f56b9b3e197e89fe Mon Sep 17 00:00:00 2001 From: lukasWuttke <54042461+LukasWodka@users.noreply.github.com> Date: Thu, 30 Jul 2026 16:43:51 +0200 Subject: [PATCH 1/3] fix(install): match a quoted fish path, spaces included (#439) * 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. --- scripts/install.sh | 41 +++++++++++++++++++++- scripts/tests/install-verify.sh | 60 +++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/scripts/install.sh b/scripts/install.sh index e1507fe..12d32e7 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -648,7 +648,46 @@ rc_lists_dir() { { n = 0 if ($0 ~ /(^|[^A-Za-z_])fish_add_path([^A-Za-z_]|$)/) { - for (i = 1; i <= NF; i++) if ($i !~ /^-/ && $i !~ /fish_add_path/) cand[++n] = $i + # 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) diff --git a/scripts/tests/install-verify.sh b/scripts/tests/install-verify.sh index 1b42ca7..58d30a1 100755 --- a/scripts/tests/install-verify.sh +++ b/scripts/tests/install-verify.sh @@ -469,6 +469,66 @@ for spec in "zsh:.zshrc:export PATH=" "fish:.config/fish/config.fish:fish_add_pa 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 ] From 45b2a2988a06f08e6fca719e6e0b155c3246c167 Mon Sep 17 00:00:00 2001 From: lukasWuttke <54042461+LukasWodka@users.noreply.github.com> Date: Fri, 31 Jul 2026 09:27:18 +0200 Subject: [PATCH 2/3] chore: add redacted gitleaks history baseline (#436) 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 --- .github/workflows/code-quality-caller.yml | 1 + .gitleaks-baseline.json | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 .gitleaks-baseline.json diff --git a/.github/workflows/code-quality-caller.yml b/.github/workflows/code-quality-caller.yml index eb1e992..3600fcc 100644 --- a/.github/workflows/code-quality-caller.yml +++ b/.github/workflows/code-quality-caller.yml @@ -31,3 +31,4 @@ jobs: # fleet-wide + advisory soak done (backend#1303). soft-fail: false all-files: ${{ inputs.all-files || false }} + gitleaks-baseline: .gitleaks-baseline.json 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" + } +] From be16f6fc506de89baf4355064d5a04ffcc88215c Mon Sep 17 00:00:00 2001 From: lukasWuttke <54042461+LukasWodka@users.noreply.github.com> Date: Fri, 31 Jul 2026 09:53:11 +0200 Subject: [PATCH 3/3] chore: re-baseline the PR template on the org one, keeping repo-specific checks (#440) --- .github/pull_request_template.md | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) 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