analyze: detect stray-apostrophe and unescaped-${VAR} Woodpecker footguns #12
Reference in New Issue
Block a user
Delete Branch "footgun-detection"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes oleks/pipetree#11.
Adds two static checks to
-analyze(internal/analyze/footguns.go), alongside the existing shared-image/broken-depends_on checks:-c bash -euxc '...'-style single-quoted shell block — including inside a#comment — closes the quote early and hands the rest of the script to the outer shell to re-tokenize. Live incident: oleks/mempalace pipeline 218 failedexit 127("patch: command not found", though patch WAS installed) because a comment contained the word "Woodpecker's" — root-caused and fixed in oleks/mempalace commit54899ec. The same idiom had already been bitten once before (e409e70).${VAR}in the raw pipeline text at parse time, before the shell runs — including inside comments. A shell variable meant for the shell needs to be escaped as$${VAR}(oleks/mempalace commit33b880d). Woodpecker's own${CI_*}builtins and a pipeline's ownmatrix:axis names are excluded as legitimate unescaped uses.Both checks are conservative by design to avoid false positives:
'); commands that don't match this precise shape are left unchecked.CI_*builtins and, after a real false positive found during fleet validation (see below), the pipeline's ownmatrix:axis names too.Testing
go build ./...andgo test ./...(single small binary, no full-repo lint per the build-delegation policy). New tests ininternal/analyze/footguns_test.gocover:testdata/mempalace-stray-apostrophe.yamlis the actual pipeline-218-broken commit (54899ec^),testdata/mempalace-unescaped-var.yamlis the actual pre-33b880dcommit.testdata/mempalace-fixed.yaml(today's live, post-fix.woodpecker/test.yaml),testdata/matrix-fixture.yaml(a realmatrix:pipeline, guards the false positive below), plus unit tests for$$-escaping,'\''-escaping, and comment-embedded footguns.Fleet validation (
~/projects)First run found a real false positive:
${TARGET_ARCH}in oleks/ii-researcher, oleks/csi-s3, and oleks/common-chronicle (18 hits) — all three declarematrix: {TARGET_ARCH: [...]}and reference${TARGET_ARCH}unescaped by design (Woodpecker's matrix feature works by substituting${AXIS_NAME}before parsing). Excluded a pipeline's own matrix axis names and re-ran:${BUILDKIT_ADDR}/${HP}, shell vars set via stepenvironment:/local assignment), oleks/terminal-agent (${wait}, a local retry-counter shell var)..woodpecker/test.yaml: clean, confirming no false positives on the two repos known-good post-fix.Test plan
go build ./...go test ./...gofmt -l .cleanpipetree -C ~/projects -analyzeagainst the real fleet and reviewed every finding by handAdds two static checks to -analyze: a literal apostrophe inside a -c bash -euxc '...' single-quoted shell block (closes the quote early, dropping the rest of the script to the outer shell - this broke oleks/mempalace pipeline 218 via a comment containing "Woodpecker's"), and an unescaped ${VAR} in step commands (Woodpecker substitutes braced ${VAR} at parse time before the shell runs, so shell-owned vars need $${VAR} - see oleks/mempalace commit 33b880d). Woodpecker's own ${CI_*} builtins and a pipeline's own matrix: axis names are excluded as legitimate unescaped uses (the matrix exclusion was added after a fleet run initially false-positived on oleks/ii-researcher/csi-s3/ common-chronicle's ${TARGET_ARCH} matrix axis). Verified against ~/projects: 0 stray apostrophes fleet-wide, 8 genuine unescaped-var findings (oleks/deals-site, oleks/element-web-patched, oleks/terminal-agent); oleks/oracle-adb-backend and the fixed oleks/mempalace test.yaml are clean, confirming no false positives on the two repos known-good post-fix.