From 0d62a3b573362c8ad7091c031cfcd452ffaf1775 Mon Sep 17 00:00:00 2001 From: avidal Date: Tue, 7 Apr 2026 10:09:40 -0500 Subject: [PATCH] fix: encode binary content over rest api with base64 instead of utf-8 In the switch to v3, when the REST API was brought in, there was a bug where binary content was pushed via the REST API using utf-8 encoding instead of base64, which then triggered a downstream issue: the action release process uses commit-headless to push the built binaries into `dist/`, and these binaries were then treated as shell scripts. --- .github/workflows/test.yml | 77 +++++++++++++++++++++++++++++-------- github.go | 4 +- github_test.go | 78 ++++++++++++++++++++++++++++++++++++++ version.go | 2 +- 4 files changed, 143 insertions(+), 18 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ffe9b34..c303464 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -39,7 +39,7 @@ jobs: go build -buildvcs=false -o ./action-template/dist/commit-headless-linux-amd64 . - name: Upload action - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 with: name: action path: action-template/ @@ -56,7 +56,7 @@ jobs: fetch-depth: 0 - &download - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: name: action path: action/ @@ -103,6 +103,17 @@ jobs: exit 1 fi + - &verify-no-tmp-branches + name: Verify no leftover working branches + run: | + tmp_branches=$(git ls-remote --heads origin | grep -- '${{ env.TEST_BRANCH }}--headless-tmp-' || true) + if [ -n "$tmp_branches" ]; then + echo "ERROR: leftover working branches found:" + echo "$tmp_branches" + exit 1 + fi + echo "No leftover working branches" + - &cleanup name: Cleanup if: always() @@ -163,16 +174,7 @@ jobs: exit 1 fi - - &verify-no-tmp-branches - name: Verify no leftover working branches - run: | - tmp_branches=$(git ls-remote --heads origin | grep -- '${{ env.TEST_BRANCH }}--headless-tmp-' || true) - if [ -n "$tmp_branches" ]; then - echo "ERROR: leftover working branches found:" - echo "$tmp_branches" - exit 1 - fi - echo "No leftover working branches" + - *verify-no-tmp-branches - *cleanup @@ -184,7 +186,6 @@ jobs: - *checkout - *download - *configure-git - - *create-branch - name: Sync local with remote @@ -198,6 +199,7 @@ jobs: branch: ${{ env.TEST_BRANCH }} command: push + - *verify-no-tmp-branches - *cleanup test-force: @@ -339,6 +341,7 @@ jobs: exit 1 fi + - *verify-no-tmp-branches - *cleanup test-commit: @@ -349,7 +352,6 @@ jobs: - *checkout - *download - *configure-git - - *create-branch - name: Stage changes @@ -393,6 +395,7 @@ jobs: message: "this should not appear" command: commit + - *verify-no-tmp-branches - *cleanup test-modes: @@ -403,7 +406,6 @@ jobs: - *checkout - *download - *configure-git - - *create-branch - name: Create executable @@ -436,6 +438,50 @@ jobs: - *verify-no-tmp-branches - *cleanup + test-binary-content: + runs-on: ubuntu-latest + needs: build + env: *env + steps: + - *checkout + - *download + - *configure-git + - *create-branch + + - name: Stage binary from dist + run: | + cp action/dist/commit-headless-linux-amd64 ./test-binary + chmod +x test-binary + sha256sum test-binary > expected-checksum.txt + echo "Local binary checksum:" + cat expected-checksum.txt + + git add test-binary expected-checksum.txt + git commit -m "test: add binary executable" + + - name: Push binary + uses: ./action/ + with: + branch: ${{ env.TEST_BRANCH }} + command: push + + - name: Verify binary content is not corrupted + run: | + git fetch origin ${{ env.TEST_BRANCH }} + git checkout origin/${{ env.TEST_BRANCH }} -- test-binary expected-checksum.txt + + echo "Remote binary checksum:" + sha256sum test-binary + + if ! sha256sum -c expected-checksum.txt; then + echo "ERROR: binary content was corrupted during push" + exit 1 + fi + echo "Binary content integrity verified" + + - *verify-no-tmp-branches + - *cleanup + test-replay: runs-on: ubuntu-latest needs: build @@ -568,4 +614,5 @@ jobs: echo "Single commit replay test passed" + - *verify-no-tmp-branches - *cleanup diff --git a/github.go b/github.go index 8ebc5b2..3ae0f1d 100644 --- a/github.go +++ b/github.go @@ -393,8 +393,8 @@ func (c *Client) prepTree(ctx context.Context, headCommit string, change Change) // Deletion: SHA must be empty string for go-github to omit it } else { blob, _, err := c.git.CreateBlob(ctx, c.owner, c.repo, github.Blob{ - Content: github.Ptr(string(fe.Content)), - Encoding: github.Ptr("utf-8"), + Content: github.Ptr(base64.StdEncoding.EncodeToString(fe.Content)), + Encoding: github.Ptr("base64"), }) if err != nil { return "", fmt.Errorf("create blob for %s: %w", path, err) diff --git a/github_test.go b/github_test.go index 2659042..ba09bb8 100644 --- a/github_test.go +++ b/github_test.go @@ -2,6 +2,7 @@ package main import ( "context" + "encoding/base64" "encoding/json" "fmt" "io" @@ -744,6 +745,83 @@ func TestPushChanges(t *testing.T) { }) } +func TestRESTBlobUsesBase64Encoding(t *testing.T) { + // Regression test: binary file content must be base64-encoded when creating + // blobs via the REST API. Using utf-8 encoding corrupts binary data (e.g. ELF + // executables) because invalid UTF-8 sequences are mangled during string + // conversion and JSON marshaling. + binaryContent := []byte{0x7f, 'E', 'L', 'F', 0x02, 0x01, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0xFF, 0xFE, 0x80, 0x90} + + var capturedEncoding, capturedContent string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + switch { + case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/commits/"): + json.NewEncoder(w).Encode(github.Commit{ + SHA: github.Ptr("parent-sha"), + Tree: &github.Tree{SHA: github.Ptr("parent-tree-sha")}, + }) + + case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/blobs": + var req struct { + Content string `json:"content"` + Encoding string `json:"encoding"` + } + json.NewDecoder(r.Body).Decode(&req) + capturedEncoding = req.Encoding + capturedContent = req.Content + + w.WriteHeader(http.StatusCreated) + json.NewEncoder(w).Encode(github.Blob{SHA: github.Ptr("blob-sha")}) + + case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/trees": + w.WriteHeader(http.StatusCreated) + json.NewEncoder(w).Encode(github.Tree{SHA: github.Ptr("tree-sha")}) + + default: + w.WriteHeader(http.StatusOK) + json.NewEncoder(w).Encode(map[string]string{}) + } + })) + defer server.Close() + + client := newTestClient(t, server) + change := Change{ + entries: map[string]FileEntry{ + "binary-file": {Content: binaryContent, Mode: "100755"}, + }, + } + + _, err := client.prepTree(context.Background(), "parent-sha", change) + if err != nil { + t.Fatalf("prepTree failed: %v", err) + } + + if capturedEncoding != "base64" { + t.Errorf("expected blob encoding 'base64', got %q", capturedEncoding) + } + + expectedContent := base64.StdEncoding.EncodeToString(binaryContent) + if capturedContent != expectedContent { + t.Errorf("blob content mismatch\n got: %q\n want: %q", capturedContent, expectedContent) + } + + // Verify round-trip: decoding the sent content must yield the original bytes + decoded, err := base64.StdEncoding.DecodeString(capturedContent) + if err != nil { + t.Fatalf("failed to decode captured content: %v", err) + } + if len(decoded) != len(binaryContent) { + t.Fatalf("decoded length %d != original length %d", len(decoded), len(binaryContent)) + } + for i := range binaryContent { + if decoded[i] != binaryContent[i] { + t.Errorf("byte %d: got 0x%02x, want 0x%02x", i, decoded[i], binaryContent[i]) + } + } +} + func TestChooseStrategy(t *testing.T) { tests := []struct { name string diff --git a/version.go b/version.go index be23939..f32825f 100644 --- a/version.go +++ b/version.go @@ -1,3 +1,3 @@ package main -const VERSION = "3.1.1" +const VERSION = "3.2.0"