From 5801e76680958fa5397950b11cf55dfec040151d Mon Sep 17 00:00:00 2001 From: AlexeyShalaev <75322386+AlexeyShalaev@users.noreply.github.com> Date: Tue, 6 Oct 2026 09:02:17 +0300 Subject: [PATCH 1/2] fix(ci): the first runs on GitHub's Windows, macOS and image runners - test repositories take paths NTFS cannot hold (Git for Windows refused a tab and a quote in fast-import) and run no background gc (on macOS receive-pack's gc still wrote while a test removed its directory) - image checks capture docker's output before grep -q: under pipefail the early exit broke docker's pipe and failed the step - a timed fuzz run that ends with "context deadline exceeded" and no failing input is a warning --- .github/workflows/ci.yml | 23 ++++++++++++++++++---- .github/workflows/publish.yml | 8 ++++++-- internal/gitx/target_helpers_test.go | 9 ++++++--- internal/platform/fake/gitrepo.go | 9 ++++++--- internal/platform/github/ghfake/gitrepo.go | 7 +++++-- scripts/e2e/template-lib.sh | 4 +++- 6 files changed, 45 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3eae825..c4388a3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -372,8 +372,12 @@ jobs: run: | set -euo pipefail bash scripts/release/check-image.sh "$LOCAL_IMAGE@$DIGEST" "$TAG" - docker buildx imagetools inspect "$LOCAL_IMAGE@$DIGEST" --format '{{ json .SBOM }}' | grep -q '"SPDX"' - docker buildx imagetools inspect "$LOCAL_IMAGE@$DIGEST" --format '{{ json .Provenance }}' | grep -q '"SLSA"' + # Captured first: grep -q stops reading at the first match, and + # under pipefail docker's broken pipe would fail the step. + sbom="$(docker buildx imagetools inspect "$LOCAL_IMAGE@$DIGEST" --format '{{ json .SBOM }}')" + grep -q '"SPDX"' <<<"$sbom" + provenance="$(docker buildx imagetools inspect "$LOCAL_IMAGE@$DIGEST" --format '{{ json .Provenance }}')" + grep -q '"SLSA"' <<<"$provenance" # The Dockerfile's own build from source, which docs and # contributors use, passes the same checks. docker buildx build --load -t touchmark:source --build-arg VERSION="$TAG" . @@ -465,7 +469,9 @@ jobs: go-version: '1.26.x' cache: true - # Every Fuzz* target in every package runs for 30 seconds. + # Every Fuzz* target in every package runs for 30 seconds. Go's fuzzer + # sometimes ends a timed run with "context deadline exceeded" although + # no input failed: that is a warning; a failing input is an error. - name: go test -fuzz run: | set -euo pipefail @@ -473,7 +479,16 @@ jobs: listed="$(go test -list '^Fuzz' "$pkg")" for target in $(printf '%s\n' "$listed" | grep '^Fuzz' || true); do echo "::group::$pkg $target" - go test -run '^$' -fuzz "^${target}\$" -fuzztime 30s "$pkg" + if ! go test -run '^$' -fuzz "^${target}\$" -fuzztime 30s "$pkg" > fuzz.log 2>&1; then + cat fuzz.log + if grep -q 'context deadline exceeded' fuzz.log && ! grep -q 'Failing input written to' fuzz.log; then + echo "::warning::$pkg $target: the fuzzer stopped at its deadline with no failing input" + else + exit 1 + fi + else + cat fuzz.log + fi echo "::endgroup::" done done diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 955d8d1..ef008eb 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -475,8 +475,12 @@ jobs: test "$got" = "$DIGEST" || { echo "::error::$IMAGE:$t names $got, not $DIGEST"; exit 1; } done bash scripts/release/check-image.sh "$IMAGE:$VERSION" "$TAG" - docker buildx imagetools inspect "$IMAGE@$DIGEST" --format '{{ json .SBOM }}' | grep -q '"SPDX"' - docker buildx imagetools inspect "$IMAGE@$DIGEST" --format '{{ json .Provenance }}' | grep -q '"SLSA"' + # Captured first: grep -q stops reading at the first match, and + # under pipefail docker's broken pipe would fail the step. + sbom="$(docker buildx imagetools inspect "$IMAGE@$DIGEST" --format '{{ json .SBOM }}')" + grep -q '"SPDX"' <<<"$sbom" + provenance="$(docker buildx imagetools inspect "$IMAGE@$DIGEST" --format '{{ json .Provenance }}')" + grep -q '"SLSA"' <<<"$provenance" - name: The release's files against the signed checksums.txt if: runner.arch == 'X64' diff --git a/internal/gitx/target_helpers_test.go b/internal/gitx/target_helpers_test.go index 97603a1..17b6f9f 100644 --- a/internal/gitx/target_helpers_test.go +++ b/internal/gitx/target_helpers_test.go @@ -96,11 +96,14 @@ func newServed(t *testing.T, root, name string) *served { if _, err := g.Run(t.Context(), nil, "init", "-q", "--bare", "-b", "main", dir); err != nil { t.Fatal(err) } - // Appended rather than set with git config: three git runs fewer per - // repository, which counts on Windows. + // Appended rather than set with git config: fewer git runs per + // repository, which counts on Windows. No garbage collection runs behind + // a test's back (receive-pack starts one after a push, and on macOS it + // was still writing when the test removed its directory), and Git for + // Windows takes paths NTFS cannot hold, since nothing is checked out. f, err := os.OpenFile(filepath.Join(dir, "config"), os.O_APPEND|os.O_WRONLY, 0) if err == nil { - _, err = f.WriteString("[uploadpack]\n\tallowFilter = true\n\tallowAnySHA1InWant = true\n[http]\n\treceivepack = true\n") + _, err = f.WriteString("[core]\n\tprotectNTFS = false\n[uploadpack]\n\tallowFilter = true\n\tallowAnySHA1InWant = true\n[http]\n\treceivepack = true\n[receive]\n\tautogc = false\n[gc]\n\tauto = 0\n") err = errors.Join(err, f.Close()) } if err != nil { diff --git a/internal/platform/fake/gitrepo.go b/internal/platform/fake/gitrepo.go index 31a74b9..a45130e 100644 --- a/internal/platform/fake/gitrepo.go +++ b/internal/platform/fake/gitrepo.go @@ -46,9 +46,12 @@ var ( const gitTimeout = 2 * time.Minute // repoConfig is appended to the config of every bare repository: partial -// clones and fetches by id are served, pushes are accepted over HTTP, and -// no garbage collection runs behind a test's back. -const repoConfig = `[uploadpack] +// clones and fetches by id are served, pushes are accepted over HTTP, no +// garbage collection runs behind a test's back, and Git for Windows takes +// paths NTFS cannot hold (a tab, a quote), since nothing is checked out. +const repoConfig = `[core] + protectNTFS = false +[uploadpack] allowFilter = true allowAnySHA1InWant = true [http] diff --git a/internal/platform/github/ghfake/gitrepo.go b/internal/platform/github/ghfake/gitrepo.go index c226084..27f6c25 100644 --- a/internal/platform/github/ghfake/gitrepo.go +++ b/internal/platform/github/ghfake/gitrepo.go @@ -32,8 +32,11 @@ const ( // repoConfig is appended to the config of every bare repository: partial // clones and fetches by id are served, pushes are accepted over HTTP, // refs/pull/* are hidden from pushes as on GitHub ("deny updating a hidden -// ref"), and no garbage collection runs behind a test's back. -const repoConfig = `[uploadpack] +// ref"), no garbage collection runs behind a test's back, and Git for +// Windows takes paths NTFS cannot hold, since nothing is checked out. +const repoConfig = `[core] + protectNTFS = false +[uploadpack] allowFilter = true allowAnySHA1InWant = true [http] diff --git a/scripts/e2e/template-lib.sh b/scripts/e2e/template-lib.sh index 0c6d8ee..6750549 100644 --- a/scripts/e2e/template-lib.sh +++ b/scripts/e2e/template-lib.sh @@ -50,7 +50,9 @@ tpl_source=${tpl_source:-} # TEXT; it fails when the container stops or the time is up. tpl_wait_log() { local c=$1 text=$2 deadline=$((SECONDS + $3)) state - until docker logs "$c" 2>&1 | grep -qF -- "$text"; do + # The log is captured first: with pipefail, grep -q's early exit would + # fail docker logs on a broken pipe, and the wait would never end. + until grep -qF -- "$text" <<<"$(docker logs "$c" 2>&1)"; do if state=$(docker inspect -f '{{.State.Running}}' "$c" 2>/dev/null) && [ "$state" != true ]; then docker logs --tail 50 "$c" >&2 || true die "$c stopped before it printed \"$text\"" From d9cac8166a90fa287f89f485d5f6214beede9c8e Mon Sep 17 00:00:00 2001 From: AlexeyShalaev <75322386+AlexeyShalaev@users.noreply.github.com> Date: Tue, 6 Oct 2026 09:20:33 +0300 Subject: [PATCH 2/2] fix(prbody): a path with backticks no longer closes the decline comment's YAML block The fence around the suggested ignore list is longer than any run of backticks inside it. Found by FuzzDeclinedComment on CI. --- internal/prbody/comments.go | 14 ++++++++++---- internal/prbody/comments_test.go | 24 +++++++++++++++++------- internal/prbody/fuzz_test.go | 1 + 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/internal/prbody/comments.go b/internal/prbody/comments.go index f410a63..1a789c5 100644 --- a/internal/prbody/comments.go +++ b/internal/prbody/comments.go @@ -175,20 +175,26 @@ func DeclinedComment(pr int64, paths []string, optInFile string) string { if len(ps) == 0 { return b.String() } - b.WriteString("\n\nTo keep your own versions of these files for good, add them to `ignore` in " + optIn + ":\n\n```yaml\nignore:\n") + var y strings.Builder + y.WriteString("ignore:\n") listed, size := 0, 0 for _, p := range ps { entry := " - " + yamlQuote(ignorePattern(p)) + "\n" if listed == maxRows || (listed > 0 && size+len(entry) > maxYAML) { break } - b.WriteString(entry) + y.WriteString(entry) listed, size = listed+1, size+len(entry) } if more := len(ps) - listed; more > 0 { - b.WriteString(" # …and " + strconv.Itoa(more) + " more\n") + y.WriteString(" # …and " + strconv.Itoa(more) + " more\n") } - b.WriteString("```") + // A path may hold backticks: the fence is longer than any run of them + // inside, so a path can never close the block early. + fence := strings.Repeat("`", max(3, longestRun(y.String(), '`')+1)) + b.WriteString("\n\nTo keep your own versions of these files for good, add them to `ignore` in " + optIn + ":\n\n" + fence + "yaml\n") + b.WriteString(y.String()) + b.WriteString(fence) return b.String() } diff --git a/internal/prbody/comments_test.go b/internal/prbody/comments_test.go index c12d68a..f6349bc 100644 --- a/internal/prbody/comments_test.go +++ b/internal/prbody/comments_test.go @@ -317,15 +317,25 @@ func checkComment(t *testing.T, s string) { // yamlBlock returns the YAML inside the fenced block of a comment. func yamlBlock(t *testing.T, s string) string { t.Helper() - _, rest, ok := strings.Cut(s, "```yaml\n") - if !ok { - t.Fatalf("no YAML block in\n%s", s) - } - block, _, ok := strings.Cut(rest, "```") - if !ok { + // The opening line is a fence of three or more backticks and "yaml"; + // the block ends at a line holding that fence alone. + lines := strings.SplitAfter(s, "\n") + for i, line := range lines { + fence, ok := strings.CutSuffix(strings.TrimSuffix(line, "\n"), "yaml") + if !ok || len(fence) < 3 || strings.Trim(fence, "`") != "" { + continue + } + var b strings.Builder + for _, line := range lines[i+1:] { + if strings.TrimSuffix(line, "\n") == fence { + return b.String() + } + b.WriteString(line) + } t.Fatalf("an open YAML block in\n%s", s) } - return block + t.Fatalf("no YAML block in\n%s", s) + return "" } // yamlIgnore parses the YAML block of a comment. diff --git a/internal/prbody/fuzz_test.go b/internal/prbody/fuzz_test.go index e303cd9..6286196 100644 --- a/internal/prbody/fuzz_test.go +++ b/internal/prbody/fuzz_test.go @@ -180,6 +180,7 @@ func FuzzDeclinedComment(f *testing.F) { f.Add(int64(12), "AGENTS.md\x00docs/[draft].md\x00@x/y.md", optInName) f.Add(int64(0), hostileSeed, "@opt`in") f.Add(int64(-3), " \x00/\x00./a\x00a*b?c\x00\t tab", "") + f.Add(int64(12), "```\x00a````b.md", "0") // backticks in a path once closed the YAML block early f.Fuzz(func(t *testing.T, pr int64, paths, optIn string) { ps := splitN(paths, 300) got := DeclinedComment(pr, ps, optIn)