feat(quality): shared code-quality reusable workflow + house-rules checker by LukasWodka · Pull Request #65 · tracebloc/.github

LukasWodka · 2026-07-25T07:53:49Z

What

The org's first shared code-quality reusable workflow. All 16 existing reusable workflows in this repo automate process (kanban, FR gate, WIP, PII); none of them looks at what the code does. This fills that gap using the delivery pattern this repo has already proven 16 times.

.github/workflows/code-quality.yml — workflow_call, typed inputs with defaults, contents: read only
scripts/house-rules.sh — the portable checker the house-rules job runs

Refs tracebloc/backend#930 (execution item 2, and item 1 for the grep-level learned rules).

Why

Measured over 60 days of automated code review: a 3% false-positive rate, so what the reviewer reports is real — and it arrives at the most expensive moment. Of the hand-classified findings, 20% were expressible as a lint or grep rule and 14% were rules the team had already agreed on, being re-enforced one PR at a time by a reviewer instead of once by CI. A type checker would have caught 1 of 120, which is why mypy is not here.

Jobs

Four separate jobs, so a ruff failure never hides what shellcheck found (steps inside one job would short-circuit; jobs do not).

Job	Gate	Notes
`ruff`	`python: true`	Uses the repo's own ruff config when it has one. Otherwise a deliberately small default selection (`E4,E7,E9,F` — pyflakes + the pycodestyle errors that mean a real bug). No formatting-opinion families (E1/E2/E3, W, I, D, ANN): those report thousands of findings on legacy code and get the job deleted. Version pinned.
`shellcheck`	`shell: true`	Preinstalled on `ubuntu-latest` — no download, no third-party action. `shellcheck-severity` defaults to `error`; `warning` is the documented next step.
`gitleaks`	on by default	Credential scanning, where the org had none. Scans exactly the commit range the PR adds, so a value added and then removed again is still found. `--redact` everywhere. Supports `.gitleaks.toml` and a baseline.
`house-rules`	on by default	`--tlsv1.2` on curl; timeouts on curl / `kubectl wait,rollout status,delete` / `helm` when it waits; pipelines in scripts with no `set -o pipefail`.

Supply chain

gitleaks is installed from its release tarball pinned by version and verified against a pinned SHA-256, not via gitleaks-action. Two reasons: gitleaks-action requires a GITLEAKS_LICENSE for organization-owned repos and would simply fail here, and a version+digest pin on the artefact we actually execute is stronger than a commit pin on a wrapper that downloads it for us. actions/checkout is pinned to a commit SHA.

Rollout safety — `soft-fail` defaults to `true`

Per the settled decision on #930 ("yes, a deterministic gate may block a merge, but land non-required first"): findings are annotated on the diff and written to $GITHUB_STEP_SUMMARY, and the job exits 0. This is a migration setting, not a destination — the header documents the path:

add the caller with defaults → findings appear, nothing blocks
run once with all-files: true to size the backlog
clear it (or record a gitleaks baseline / add ignore pragmas)
flip soft-fail: false
mark Code quality / <job> required in branch protection

A repo still on the default months from now is itself the finding.

The checker is a lexer, not a grep

Shell cannot be linted with grep: curl appears in comments, in heredoc bodies, inside quoted strings, as curlimages/curl, and as an argument to command -v. So house-rules.sh (POSIX sh + awk, no jq/yq/python) does quote-aware comment stripping, skips heredoc bodies, joins \-continuations while reporting the original line, descends into $( … ) even inside double quotes, matches tool names only in command position, and resolves flag-holding variables so curl -fsSL $CURL_SECURE "$url" satisfies the TLS rule.

Deliberate precision choices, documented in the script rather than hidden: the pipefail rule skips POSIX-sh scripts (set -o pipefail is not POSIX — demanding it there is wrong advice) and skips files that another file sources (a library inherits its entrypoint's shell options).

Validation

client/scripts (29 files): 20 findings, all true positives — e.g. lib/gpu-amd.sh:29curl -fsSL "$dir_url" | grep … with neither TLS floor nor timeout. Zero kubectl/helm findings, correctly: every kubectl wait/rollout status there already carries --timeout, including the ones split across a line continuation.
cli/scripts (9 files): 9 findings, all true positives — every installer curl has --tlsv1.2 and none has a timeout, the exact mirror image of client.
One false-positive class was found and fixed before opening this PR: readonly CURL_SECURE="--tlsv1.2" used as curl $CURL_SECURE … was reported as missing the flag — 5 findings across 4 files, the org's most common correct hardening idiom being the checker's most common false positive. Variable resolution now handles it.
Fixture pair covering every rule and every FP guard (comments, ${var#prefix}, command -v curl, curlimages/curl, | inside a quoted regex, heredoc bodies, line continuations, non-blocking subcommands, pragmas).
actionlint: clean on the new file. YAML parses. shellcheck -s sh and -s bash: clean. Identical results under dash and under BSD awk. 0.5s for a whole-repo scan.

Extending it, per repo, without touching this workflow

.house-rules.conf at the repo root:

exclude: third_party/*
timeout-wrapper: guard # a wrapper that already bounds time
disable: curl-timeout
rule: no-print | *.py | ^[[:space:]]*print\( | use client_logger, not print()

rule: lines work on any language, so the remaining learned rules (list() not .all(), client_logger not print_and_log) can be added per repo without a change here. One line can be silenced with # house-rules: ignore=<rule-id>.

Local run, identical to CI: ./house-rules.sh --all (--help documents every rule, exclusion and pragma).

Merge dependency — read before adopting

No callers are added, here or in any other repo. A thin caller referencing code-quality.yml@main cannot resolve until this workflow exists on main, so callers can only be added after this merges to develop and is promoted develop → staging → main on the normal cadence. The exact caller YAML to copy is in the workflow's header comment.

Nothing in this PR runs on this repo yet.

Test plan

actionlint clean on code-quality.yml (remaining findings in the run are all pre-existing, in other workflows)
YAML parse of every workflow in the repo
shellcheck -s sh and -s bash clean on house-rules.sh
Checker exercised against real client and cli installer scripts; every finding manually verified
FP fixture silent; rule fixture fires on all rules
Portability: identical output under dash and BSD awk
First real CI run happens on the first adopting repo's PR, after this reaches main

🤖 Generated with Claude Code

Note

Low Risk
New additive CI assets with no callers in-repo, minimal contents: read permissions, and soft-fail defaults—low blast radius until adopters flip blocking mode.

Overview
Introduces the org’s first reusable workflow that inspects code, not just process. Repos adopt it via a thin caller; nothing in this PR wires it up here, so behavior changes only after merge and a separate caller lands elsewhere.

code-quality.yml exposes four independent jobs (failures don’t hide each other): optional ruff (python: true) and shellcheck (shell: true); default-on gitleaks (commit-range or full history, baseline support, SHA-pinned binary install) and house-rules (checks out tracebloc/.github for scripts/house-rules.sh). Shared inputs include soft-fail: true by default (annotate + job summary, exit 0), all-files for backlog sizing, and PR-scoped diffs with fallback to full scan when git diff fails so gates don’t silently no-op.

house-rules.sh is a POSIX sh + awk shell lexer (not plain grep) enforcing org rules: curl TLS/timeouts, helm --wait without --timeout, pipefail on risky pipelines, plus per-repo .house-rules.conf custom rule: lines and ignore pragmas.

Adoption path documented in the workflow header: report-only → clear backlog → soft-fail: false → required checks.

^{Reviewed by Cursor Bugbot for commit f3a17e1. Bugbot is set up for automated code reviews on this repo. Configure here.}

…ecker The org has shipped the reusable-workflow + thin-caller pattern 16 times, every one of them for process automation (kanban, FR gate, WIP, PII). This is the first one that looks at the code. Four independent jobs, so one failure never hides another's output: ruff Python lint. Input-gated. Falls back to a deliberately small default selection (E4,E7,E9,F) when the repo has no ruff config, and defers entirely to the repo's own config when it has one. No formatting-opinion families — those would report thousands of findings on legacy code and get the job removed. shellcheck Preinstalled on the runner, so no download and no third-party action. Severity configurable, default `error`. gitleaks Credential scanning, where there was none. Installed from the release tarball pinned by version AND verified against a pinned SHA-256: gitleaks-action needs a licence for organization-owned repos, and pinning the artefact we execute beats pinning a wrapper that fetches it. Scans exactly the commit range the PR adds, so a value added and then removed again is still found. house-rules scripts/house-rules.sh — POSIX sh + awk, no jq/yq/python. The grep-level subset of the rules the automated reviewer keeps re-teaching us: --tlsv1.2 on curl, timeouts on curl/kubectl/helm, and pipelines in scripts with no `set -o pipefail` (a `curl … | bash` of a 404 exits 0). Shell cannot be linted with grep, so the checker carries a small lexer: quote-aware comment stripping, heredoc bodies skipped, `\`-continuations joined while reporting the original line, descent into `$( … )` inside double quotes, tool names matched only in command position, and resolution of flag-holding variables so `curl $CURL_SECURE` satisfies the TLS rule. Verified against the real installer scripts in client and cli: 20 and 9 true findings, zero false positives, 0.5s for a whole repo. Every rule and FP guard is covered by a fixture pair, and results are identical under dash and under BSD awk. `soft-fail` defaults to true: findings are annotated on the diff and written to the job summary, and the job exits 0. Landing non-required first is the settled rollout decision — a linter switched on against an unlinted backlog gets the check removed, not the backlog fixed. The header documents the flip to `soft-fail: false` plus branch protection as the intended destination. Repos extend it without touching this workflow via `.house-rules.conf` (exclude / wrapper / timeout-wrapper / risky / disable / custom `rule:` lines that work on any language), and silence one line with a `# house-rules: ignore=<rule>` pragma. No callers are added here: `@main` callers cannot resolve until this merges to main. Refs tracebloc/backend#930 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

LukasWodka · 2026-07-25T07:54:01Z

bugbot run

`xargs -a file` splits on whitespace, so a path containing a space (they exist — client/scripts/lib has one) would be passed as two broken paths. Convert to NUL delimiters and read with `xargs -0`, which also avoids the `-d '\n'` trailing-empty-argument ambiguity between xargs implementations. `-r` stays: with an empty list, ruff would otherwise fall back to scanning the entire tree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

LukasWodka · 2026-07-25T07:56:07Z

bugbot run

LukasWodka · 2026-07-25T08:00:34Z

Independent verification of the checker (for whoever reviews this)

I ran scripts/house-rules.sh --all myself against two real repos, outside CI, and the numbers reproduce exactly:

Repo	Result
`client`	20 findings — 11 `curl-tls`, 9 `curl-timeout`
`cli`	9 findings — 8 `curl-timeout`, 1 `pipefail`

Zero kubectl/helm findings on client — correct, since every kubectl wait / rollout status there already carries --timeout, including ones split across a \-continuation. That negative result is the one I'd have most expected a grep-based checker to get wrong, so it's the best evidence the lexer is doing real work.

The findings are also independently corroborated: client's curl-tls hits match what I found by hand while filing #1252 (lib/common.sh:326/:347 — in a file that definesreadonly CURL_SECURE="--tlsv1.2" on line 10 and then doesn't use it — plus lib/gpu-amd.sh:29, which is missing both the TLS floor and any timeout).

cli is the exact mirror image and is a new finding: every installer curl carries --tlsv1.2, and none carries a timeout. That is the "unbounded external call" class (39 findings / 6% org-wide) landing in the customer-facing installer, where a hung endpoint blocks an install indefinitely.

Two design choices worth a reviewer's explicit blessing, both of which I think are right:

gitleaks by pinned binary + SHA-256 rather than the official action. The reported reason is that gitleaks-action requires a GITLEAKS_LICENSE for organization-owned repos and would fail on every private caller. Independent of whether that holds, verifying a pinned binary against a pinned checksum is a stronger supply-chain guarantee than SHA-pinning a wrapper that downloads it at runtime — so the outcome is good either way. Worth a second pair of eyes on the version/hash pair at review time.
pipefail deliberately skips POSIX-sh scripts and sourced libraries.pipefail isn't POSIX, so demanding it in a sh script is wrong advice, and a sourced library inherits its entrypoint's options. Without the second exclusion all 13 of client/scripts/lib/*.sh would be flagged for nothing. This is the kind of precision that decides whether a checker gets adopted or switched off.

On adoption: these 29 findings are the point, not a blocker. soft-fail: true means a repo can adopt the workflow immediately and clear its backlog on its own cadence — which is exactly the "land non-required first" path settled on #930. I'd rather these 29 become one visible backlog per repo than 29 more tickets.

1. Timeout hid the tool from every rule. `findtok` recognised command position after separators, shell keywords and repo-declared wrappers, but not after a time bound. So in `timeout 30 curl ...` the `curl` was invisible and `curl-tls` silently stopped applying to the exact hardening pattern the config documents as supported. Added TIMEOUT_PREFIX_RE to the command- position checks. 2. The timeout wrapper demanded a bare digit. `TIMEOUT_WRAPPER_RE` required a numeric token immediately after the wrapper name, so `timeout -k 5 30 ...`, `timeout --foreground 30 ...`, `timeout 30s ...` and the documented `timeout-wrapper: guard` form (`guard curl ...`, no duration) all failed the bounded check and still raised *-timeout findings on correctly bounded calls. Replaced with TW_ARG, which accepts flags, flag arguments, unit suffixes, variables, and the empty case. 3. The wrapper match was not end-anchored. `WRAPPER_RE` was applied unanchored to the text before a tool, unlike the shell-keyword check beside it, so any earlier wrapper name in the segment marked later tokens as command position. Anchored it to match the keyword check's shape. Verified against a fixture: before, only the bare unbounded call was detected and all five timeout-wrapped variants were invisible. After, curl-tls fires on every call missing --tlsv1.2 and curl-timeout fires only on the genuinely unbounded one. Refs #65

LukasWodka · 2026-07-26T10:43:32Z

All three Bugbot findings addressed in 5f0ff26.

#	Finding	Fix
1	Timeout hides curl from rules	`findtok` now treats a time bound as a command-position prefix (`TIMEOUT_PREFIX_RE`)
2	Timeout-wrapper requires a digit	`TW_ARG` replaces the bare-digit requirement — accepts flags, flag arguments, unit suffixes, variables, and the empty case
3	Wrapper match not end-anchored	`WRAPPER_RE` anchored to match the shape of the shell-keyword check beside it

Verified against a fixture

Five timeout-wrapped calls plus one bare unbounded call plus one fully hardened call:

Before — only the bare call was seen at all. All five timeout-wrapped variants were invisible to
every rule, so curl-tls never fired on them.

After — curl-tls fires on every call missing --tlsv1.2, and curl-timeout fires only on
the genuinely unbounded one. The hardened call stays silent.

One residual, stated rather than hidden

timeout "$DUR" curl … still isn't flagged for curl-tls. The regex itself matches that form — I
verified all six variants against TIMEOUT_PREFIX_RE directly in awk — so the remaining gap is in
how the quoted token is segmented beforefindtok sees it, not in this fix. It fails safe: a
missed finding, not a false one. Worth a follow-up rather than widening the tokenizer under a
bugfix.

Net effect on real detections: 1 → 5 on the fixture, with no false positives introduced.

bugbot run

- False positive: pf_has only matched the glued form (set -euo pipefail), so the split forms 'set -eu -o pipefail' and 'set -e -o pipefail' failed to register and safe scripts still got pipefail findings. Regex now allows earlier flag clusters before '-o pipefail'. - False negative: pf_has was tested on lraw, which still contains quoted string contents, so 'echo "... set -o pipefail ..."' marked the whole file safe and suppressed every real pipeline finding. Now tested on lmask (strings blanked). Verified against fixtures with a risky producer (curl | grep): glued and split safe forms report 0; a genuinely-missing pipefail and a string-decoy both report 1. Against origin, the split form false-positived and the string decoy false-negatived — both now correct.

LukasWodka · 2026-07-26T11:05:26Z

Both remaining Bugbot findings fixed in the latest commit.

False positive — pf_has only matched glued set -euo pipefail; set -eu -o pipefail and set -e -o pipefail slipped through and safe scripts got flagged. Regex now allows earlier flag clusters before -o pipefail.
False negative — detection read lraw (with quoted contents), so echo "... set -o pipefail ..." marked the whole file safe. Now reads lmask (strings blanked).

Verified with curl | grep fixtures: glued + split safe forms → 0 findings; genuinely-missing + string-decoy → 1 each. Against origin the split form false-positived and the decoy false-negatived; both correct now.

That's all 3 (curl/timeout) + 2 (pipefail) findings on this PR addressed.

bugbot run

…nation, quoted heredoc) - Path-less source: the extractor ran s|.*/|| before stripping the source/. keyword, so 'source helpers.sh' collapsed to 'source' and never entered $SOURCED — sourced libs still got pipefail findings. Strip the keyword first. - Custom rule alternation: rule: lines split on every | took field 3 as the ERE, truncating any pattern with alternation. Treat the LAST field as the message and rejoin 3..n-1 as the pattern; a rule message may no longer contain | (ids/globs never do). - Quoted heredoc: heredoc detection ran on lraw, so a <<TAG inside a string opened in_hd and swallowed real following lines. Match on lraw (to keep <<'EOF' tags working) but require the << to survive masking, i.e. not be inside a quoted string. Each proven against a fixture: before/after diverge in the expected direction, no regressions. shellcheck clean.

LukasWodka · 2026-07-26T11:20:16Z

Round-3 findings fixed (all three real, same parse-on-raw family as before plus a rule-parser limit):

Path-less source — s|.*/|| ran before stripping the source/. keyword, so source helpers.sh collapsed to source and the lib never entered $SOURCED. Keyword stripped first now.
ERE alternation — rule: lines split on every | truncated any ERE containing alternation. Message is now the last field; ERE rejoins the middle.
Quoted heredoc — detection on lraw let a <<TAG inside a string swallow real lines. Now requires the << to survive masking (real operator, not in a string), while still parsing <<'EOF'.

Each proven against a before/after fixture; shellcheck clean. That's 8 findings across 3 rounds on this PR now addressed.

Flagging the pattern: this is a net-new 1,325-line parser and each Bugbot round surfaces a fresh subset. It's converging (round 3 was 3, all fixed), but worth a human eye on whether the script's complexity is worth it vs a smaller ruleset.

LukasWodka · 2026-07-26T11:20:17Z

bugbot run

…tegrity house-rules.sh: - Quoted path-less source: quote stripping deleted from the first quote to EOL, so 'source "helpers.sh"' collapsed to empty and the lib never entered $SOURCED. Delete quote characters instead. - Env-assignment prefixes: 'HTTPS_PROXY=... curl' / 'KUBECONFIG=... kubectl' left the tool invisible to command-position detection, so tls/timeout/pipefail never ran on it. findtok now treats one-or-more VAR=val prefixes as command position. - Glued -m: 'curl -m30' / 'curl -fsm30' still raised curl-timeout despite a max-time being present; the short-option check now accepts a glued digit. code-quality.yml: - Ruff silently no-op'd when 'git diff' against BASE_SHA failed (empty file list -> skipped with success). Now falls back to scanning all files, matching shellcheck/house-rules — a gate must not skip itself on a bad diff. Each parser fix proven against before/after fixtures incl. a regression check that a genuinely unbounded curl still flags. shellcheck + actionlint clean.

LukasWodka · 2026-07-26T11:38:38Z

bugbot run

Rather than patch each reported instance, fixed the underlying class so adjacent variants are covered too: - Masked-arg (cmd position): findtok runs on the quote-masked line, where a quoted duration "$DUR" is blanked to '_'. TW_ARG now accepts a '_+' run, so 'timeout "$DUR" curl' keeps curl in command position. Covers the reported case plus any quoted arg between a wrapper and its tool. - externally_bounded: was a substring match, so 'myprog timeout 30' (timeout as an ARGUMENT) suppressed real curl/kubectl/helm-timeout findings. Now requires the wrapper to be in COMMAND position via findtok. - kubectl subcommand: 'wait'/'delete' were matched as any word, so a resource or file named 'wait' triggered kubectl-timeout. Now anchored to follow 'kubectl' (global flags allowed between). - Source scan: extensionless shebang libs (bin/deploy) never entered $SOURCED and still got pipefail findings. Filter now accepts extensionless names. 13-case fixture matrix covering the reported cases AND adjacent variants (timeout "$DUR", -fsm30, timeout-as-arg, kubectl get pod named 'wait') all pass; a genuinely unbounded curl and real kubectl wait/delete still flag. shellcheck + actionlint clean; dogfooded over the repo with no self-findings.

LukasWodka · 2026-07-26T11:47:05Z

bugbot run

Round-6 Bugbot: separate flag arguments ('-n ns', '--context ctx') between kubectl and the subcommand broke the match, so 'kubectl -n ns wait' — a real unbounded blocking call — was missed. The matcher now models value-taking global flags (a bare word after -n/--namespace/--context/... is that flag's value, not the subcommand). Fixture matrix: all real blocking calls flag (kubectl wait, -n ns wait, --namespace ns wait, --context c -n n wait, delete, -n ns rollout status); resource/file names (get pod wait, apply -f delete.yaml) and bounded calls (--timeout, timeout 30 kubectl wait) do not. Known residual, documented rather than hidden: 'kubectl -n wait get pods' where a namespace is literally named 'wait' still false-positives, because '-n' is regex-ambiguous between a boolean flag and a value flag — no ERE can resolve it without kubectl's own flag semantics. It is a rare, safe-direction FP (a dismissible spurious finding, not a missed hang). See PR discussion re: whether the kubectl-timeout rule is worth this irreducible ambiguity.

Decision (Lukas): remove kubectl-timeout rather than keep chasing it. It was the single biggest source of review findings — 'kubectl -n wait get' (namespace named 'wait') vs 'kubectl -n ns wait' are identical in token shape and only kubectl's own flag semantics disambiguate them, which an ERE cannot do. Every round re-surfaced a variant of that ambiguity. Removes the kubectl subcommand matcher and its header docs. Keeps the rules that detect robustly on flag presence, not CLI-grammar parsing: curl-tls, curl-timeout, helm-timeout (flag-based: --wait/--atomic without --timeout), and pipefail. kubectl stays in the pipefail producer list, so 'kubectl … | x' without pipefail is still caught. Verified: the four remaining rules fire correctly, kubectl-timeout is never emitted, the kubectl pipefail case still works; shellcheck + actionlint clean; dogfooded with no self-findings.

LukasWodka · 2026-07-26T12:07:26Z

bugbot run

LukasWodka · 2026-07-26T12:07:44Z

Dropped the kubectl-timeout rule (decision: Lukas).

It was the source of most of the review rounds. kubectl -n wait get (namespace named wait) and kubectl -n ns wait are identical in token shape — only kubectl's own flag semantics tell them apart, which a POSIX ERE cannot do. Each round re-surfaced a variant of that irreducible ambiguity, so the rule was chasing an unwinnable target.

Kept the rules that detect on flag presence, not CLI-grammar parsing: curl-tls, curl-timeout, helm-timeout (--wait/--atomic without --timeout), pipefail. kubectl stays in the pipefail producer list, so kubectl … | x without pipefail is still caught.

This is the honest fix for "we were reimplementing a shell/CLI parser in awk": remove the one rule whose correctness requires a real parser, keep the ones that don't.

Round-7 Bugbot: the custom-rule file gathering appended candidates from $CAND directly, without the exclude filter the shell-file scan applies. So documented 'exclude:' paths and the workflow's '--exclude .quality-tools/*' guard were ignored for custom rules — an excluded/vendored tree could still produce findings. Factored the exclude check into a shared is_excluded() helper used by both the shell-file scan and the custom-rule scan, so the two can't drift again. Fixtures: a custom rule now skips an excluded vendor/ file while still firing on a non-excluded one, and --exclude applies to custom-rule scanning. Rule matrix still green; shellcheck + actionlint clean; dogfooded.

LukasWodka · 2026-07-26T12:12:15Z

bugbot run

externally_bounded() asked 'is a timeout wrapper anywhere in this segment' — segment-global. So a curl nested in $() was treated as bounded whenever the OUTER command was timeout-wrapped ('timeout 30 foo $(curl x)'), suppressing a real curl-timeout on the unbounded subshell call. Replaced with tool_bounded(seg, tool): a tool is bounded only when a timeout prefix sits IMMEDIATELY before THAT occurrence (its 'pre' text ends with TIMEOUT_PREFIX_RE). This is strictly more correct and also subsumes the round-5 'timeout as an argument' case — timeout-as-arg simply isn't a prefix of any tool, so no special-casing is needed. Removed the now-dead externally_bounded() and TW_NAMES. Fixtures: nested $(curl) now flags; 'timeout 30 curl', 'timeout "$DUR" curl', 'timeout -k5 30 curl' stay bounded; bare curl and timeout-as-arg still flag; helm --wait still flags. shellcheck + actionlint clean; dogfooded. Note (out of scope): helm inside a captured $() subshell isn't detected at all, timeout or not — a pre-existing helm-in-subshell quirk, not a bounding issue.

LukasWodka · 2026-07-26T12:22:06Z

bugbot run

LukasWodka · 2026-07-26T13:16:35Z

bugbot run

Round-9 Bugbot: check_line split segments on every bare '&', so a redirection like '2>&1' cut the producer segment before the '|' was seen. 'curl … 2>&1 | grep' never reached note_pipeline with the risky tool, so the pipefail rule missed a very common form of the defect it exists to catch. A bare '&' now splits only when it backgrounds a command — not when it is part of a redirection: '2>&1'/'>&2' (previous char '>' or '<') or '&>file' (next char '>'). '&&' was already handled separately. Fixtures: 'curl 2>&1|grep', 'curl >&2|grep', 'curl &>/tmp/f|grep' all flag pipefail; real backgrounding 'curl & echo' still splits (no pipeline); plain 'curl|grep' still flags; pipefail-set stays silent. All prior matrices green; shellcheck + actionlint clean; dogfooded.

LukasWodka · 2026-07-26T13:23:05Z

bugbot run

Round-10 Bugbot: when the gitleaks commit-range scan errored (missing base.sha, bad log-opts), COUNT stayed 0 -> summary said 'Nothing detected', and with the default soft-fail:true the job exited 0. A broken credential scan looked green for the whole migration window — unlike ruff/shellcheck/house-rules, which fall back to a full scan. gitleaks uses exit 0 = clean, 1 = leaks; any other code is operational. On such a code, fall back to a full-history scan instead of reporting clean. Verified the RC branch: 0/1 -> normal, 2/126 -> fallback. actionlint clean.

LukasWodka · 2026-07-26T13:28:41Z

bugbot run

LukasWodka · 2026-07-26T13:34:40Z

bugbot run

cursor

✅ 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 f3a17e1. Configure here.}

… (backend#1252) `CURL_SECURE` was a bare constant every call site had to splice in by hand, so call sites kept losing it: seven live `curl` invocations ran with no minimum TLS version, including the POST in `verify_credentials()` that carries the client's password. These installs run on customer-managed hosts and behind TLS-inspecting proxies, which negotiate down to whatever the client permits — the reason this repo adopted an explicit floor instead of trusting curl's defaults. Add `curl_secure()` in `scripts/lib/common.sh` and route every fetch in `scripts/lib/*.sh` through it (18 call sites). The wrapper always passes `--tlsv1.2` and supplies default `--connect-timeout 30` / `--max-time 300`. Defaults are injected before `"$@"`, so a call site that wants a tighter bound still wins (curl honours the last occurrence), and a transfer that bounds itself with `--speed-limit`/`--speed-time` gets no injected `--max-time` — a hard deadline would fail a slow-but-healthy link on a large binary download. Every existing site therefore keeps its effective behaviour; seven gain the floor and nine previously unbounded ones gain a deadline. Also fixed while here: - `gpu-amd.sh` had the least-bounded curl usage in the repo — no TLS floor, no timeout, no retry. Both calls now go through the wrapper; the `.deb` download is retry-wrapped. The listing scrape deliberately is not: `retry()` reports attempts on stdout, which is that function's return value. - `install-k8s.ps1`'s WSL2 here-string had the same two nvidia-container-toolkit fetches bare. It cannot source `common.sh`, so it spells the flags out inline the way the bootstrap does. `scripts/install.sh` keeps its seven hardcoded literals: it is the trust root that fetches `common.sh`, so it cannot source the wrapper. `CURL_SECURE` stays defined and unchanged for out-of-tree callers, but nothing in the repo reads it now — the wrapper names the flag itself, so the constant can never silently reshape every fetch in the installer. Enforcement: an INTERIM third check in `scripts/check-style.sh` fails on a bare `curl`. tracebloc/.github#65 already implements this properly (a shell-aware lexer, not a grep) in a shared reusable workflow, but that workflow is not on `main` yet and cannot be referenced from here until it is. The check is marked for retirement the moment this repo adds that caller. Regenerated `scripts/manifest.sha256` (R8 supply-chain gate). Found in #399. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

… (backend#1252) (#400) * fix(installer): make the curl TLS floor structural, not per-call-site (backend#1252) `CURL_SECURE` was a bare constant every call site had to splice in by hand, so call sites kept losing it: seven live `curl` invocations ran with no minimum TLS version, including the POST in `verify_credentials()` that carries the client's password. These installs run on customer-managed hosts and behind TLS-inspecting proxies, which negotiate down to whatever the client permits — the reason this repo adopted an explicit floor instead of trusting curl's defaults. Add `curl_secure()` in `scripts/lib/common.sh` and route every fetch in `scripts/lib/*.sh` through it (18 call sites). The wrapper always passes `--tlsv1.2` and supplies default `--connect-timeout 30` / `--max-time 300`. Defaults are injected before `"$@"`, so a call site that wants a tighter bound still wins (curl honours the last occurrence), and a transfer that bounds itself with `--speed-limit`/`--speed-time` gets no injected `--max-time` — a hard deadline would fail a slow-but-healthy link on a large binary download. Every existing site therefore keeps its effective behaviour; seven gain the floor and nine previously unbounded ones gain a deadline. Also fixed while here: - `gpu-amd.sh` had the least-bounded curl usage in the repo — no TLS floor, no timeout, no retry. Both calls now go through the wrapper; the `.deb` download is retry-wrapped. The listing scrape deliberately is not: `retry()` reports attempts on stdout, which is that function's return value. - `install-k8s.ps1`'s WSL2 here-string had the same two nvidia-container-toolkit fetches bare. It cannot source `common.sh`, so it spells the flags out inline the way the bootstrap does. `scripts/install.sh` keeps its seven hardcoded literals: it is the trust root that fetches `common.sh`, so it cannot source the wrapper. `CURL_SECURE` stays defined and unchanged for out-of-tree callers, but nothing in the repo reads it now — the wrapper names the flag itself, so the constant can never silently reshape every fetch in the installer. Enforcement: an INTERIM third check in `scripts/check-style.sh` fails on a bare `curl`. tracebloc/.github#65 already implements this properly (a shell-aware lexer, not a grep) in a shared reusable workflow, but that workflow is not on `main` yet and cannot be referenced from here until it is. The check is marked for retirement the moment this repo adds that caller. Regenerated `scripts/manifest.sha256` (R8 supply-chain gate). Found in #399. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(installer): stall-bound the kubectl fetch, don't give it a deadline (Bugbot) `_fetch_kubectl` had no time bound at all, so routing it through `curl_secure` handed it the wrapper's default `--max-time 300`. kubectl is a ~50 MB binary, and this repo already documents (at `_fetch_k3d_release`, same file) that a fixed ceiling fails a slow-but-healthy link at that size — so the wrapper would have made every retry fail where the fetch previously completed. Give both fetches the same `--connect-timeout 15 --speed-limit 1024 --speed-time 60` as the k3d pair. That is also how `curl_secure` knows to skip its default deadline, and it is strictly better than before: the fetch was previously unbounded in both directions, so a mid-stream stall hung the step indefinitely. Audited the other 7 sites that now inherit the 300s default — get.docker.com, get-helm-3, the Homebrew script, stable.txt, the DMG checksum, the device-plugin manifest and the amdgpu-install package are all small text/script payloads. The only large downloads in the repo are kubectl, k3d and the Docker Desktop DMG; the latter two were already stall-bounded. Adds a bats test pinning it, since nothing covered `_fetch_kubectl` before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>

…ler (#401) * fix(installer): make the curl TLS floor structural, not per-call-site (backend#1252) `CURL_SECURE` was a bare constant every call site had to splice in by hand, so call sites kept losing it: seven live `curl` invocations ran with no minimum TLS version, including the POST in `verify_credentials()` that carries the client's password. These installs run on customer-managed hosts and behind TLS-inspecting proxies, which negotiate down to whatever the client permits — the reason this repo adopted an explicit floor instead of trusting curl's defaults. Add `curl_secure()` in `scripts/lib/common.sh` and route every fetch in `scripts/lib/*.sh` through it (18 call sites). The wrapper always passes `--tlsv1.2` and supplies default `--connect-timeout 30` / `--max-time 300`. Defaults are injected before `"$@"`, so a call site that wants a tighter bound still wins (curl honours the last occurrence), and a transfer that bounds itself with `--speed-limit`/`--speed-time` gets no injected `--max-time` — a hard deadline would fail a slow-but-healthy link on a large binary download. Every existing site therefore keeps its effective behaviour; seven gain the floor and nine previously unbounded ones gain a deadline. Also fixed while here: - `gpu-amd.sh` had the least-bounded curl usage in the repo — no TLS floor, no timeout, no retry. Both calls now go through the wrapper; the `.deb` download is retry-wrapped. The listing scrape deliberately is not: `retry()` reports attempts on stdout, which is that function's return value. - `install-k8s.ps1`'s WSL2 here-string had the same two nvidia-container-toolkit fetches bare. It cannot source `common.sh`, so it spells the flags out inline the way the bootstrap does. `scripts/install.sh` keeps its seven hardcoded literals: it is the trust root that fetches `common.sh`, so it cannot source the wrapper. `CURL_SECURE` stays defined and unchanged for out-of-tree callers, but nothing in the repo reads it now — the wrapper names the flag itself, so the constant can never silently reshape every fetch in the installer. Enforcement: an INTERIM third check in `scripts/check-style.sh` fails on a bare `curl`. tracebloc/.github#65 already implements this properly (a shell-aware lexer, not a grep) in a shared reusable workflow, but that workflow is not on `main` yet and cannot be referenced from here until it is. The check is marked for retirement the moment this repo adds that caller. Regenerated `scripts/manifest.sha256` (R8 supply-chain gate). Found in #399. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(installer): stall-bound the kubectl fetch, don't give it a deadline (Bugbot) `_fetch_kubectl` had no time bound at all, so routing it through `curl_secure` handed it the wrapper's default `--max-time 300`. kubectl is a ~50 MB binary, and this repo already documents (at `_fetch_k3d_release`, same file) that a fixed ceiling fails a slow-but-healthy link at that size — so the wrapper would have made every retry fail where the fetch previously completed. Give both fetches the same `--connect-timeout 15 --speed-limit 1024 --speed-time 60` as the k3d pair. That is also how `curl_secure` knows to skip its default deadline, and it is strictly better than before: the fetch was previously unbounded in both directions, so a mid-stream stall hung the step indefinitely. Audited the other 7 sites that now inherit the 300s default — get.docker.com, get-helm-3, the Homebrew script, stable.txt, the DMG checksum, the device-plugin manifest and the amdgpu-install package are all small text/script payloads. The only large downloads in the repo are kubectl, k3d and the Docker Desktop DMG; the latter two were already stall-bounded. Adds a bats test pinning it, since nothing covered `_fetch_kubectl` before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(installer): stop the ROCm package lookup from aborting the installer `_find_package_name` ran `curl … | grep … | head -1` as a single pipeline and returned its status. `install-k8s.sh` sources this lib under `set -euo pipefail`, so that pipeline could kill the installer two different ways: 1. A failed fetch (404, timeout, proxy block) made the command substitution non-zero, the caller's assignment inherited it, and `set -e` aborted BEFORE the friendly `[[ -z "$name" ]] && error "No amdgpu-install …"` on the next line could run. The user got a silent abort mid-GPU-step instead of an actionable message, and the RHEL major-version fallback was unreachable for the same reason. 2. `head -1` can close the pipe while grep is still writing, so grep takes SIGPIPE (141) and `pipefail` propagates that as a pipeline failure even though a filename WAS found. It only triggers when the directory index exceeds the pipe buffer, so it fails on large mirrors only. Capture the fetch and the match separately and let neither fail the function, then take the first match with `${var%%…}` so `head` leaves the pipeline entirely. The contract is unchanged — filename on stdout, nothing when not found — so emptiness remains the single signal all three callers already test. Adds scripts/tests/gpu-amd.bats (first coverage for this lib): the contract, both hazards, and a caller-shaped regression test under `set -euo pipefail`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: re-trigger standard-checks after base retarget to develop (#401) Retargeting the PR base from #400's merged branch to develop doesn't fire a pull_request event, so standard-checks (Unit tests + Lint) never ran on this head. Empty commit fires synchronize so the required checks run against develop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>

cursorBot reviewed Jul 25, 2026
View reviewed changes

Comment threadscripts/house-rules.sh
Comment threadscripts/house-rules.sh Outdated
Comment threadscripts/house-rules.sh

This was referenced Jul 25, 2026
ci(actionlint): lint this repo's own workflows + clear the 29 pre-existing findings #66
Merged
fix(installer): make the curl TLS floor structural, not per-call-site (backend#1252) tracebloc/client#400
Merged

LukasWodka self-assigned this Jul 25, 2026