Skip to content

Commit 7a71d4b

Browse files
committed
Improve code quality, test coverage, and changelog links
- Fix error wrapping in upgrade.go (%v -> %w for unwrappable errors) - Remove unnecessary sync.RWMutex from markdown parser; collect headings via local state for safe concurrent Parse calls - Add comprehensive TestVerifyChecksum tests for upgrade checksum verification - Add TestValidateRedirectTarget tests for PlantUML SSRF prevention - Guard .gitattributes read with stat + size check in GitHub source - Complete changelog compare links for v0.6.4–v0.6.7
1 parent 4a779cd commit 7a71d4b

File tree

7 files changed

+357
-38
lines changed

7 files changed

+357
-38
lines changed

CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,11 @@ All notable changes to this project will be documented in this file. The format
584584

585585
---
586586

587-
[Unreleased]: https://github.com/yeasy/mdpress/compare/v0.6.3...HEAD
587+
[Unreleased]: https://github.com/yeasy/mdpress/compare/v0.6.7...HEAD
588+
[0.6.7]: https://github.com/yeasy/mdpress/compare/v0.6.6...v0.6.7
589+
[0.6.6]: https://github.com/yeasy/mdpress/compare/v0.6.5...v0.6.6
590+
[0.6.5]: https://github.com/yeasy/mdpress/compare/v0.6.4...v0.6.5
591+
[0.6.4]: https://github.com/yeasy/mdpress/compare/v0.6.3...v0.6.4
588592
[0.6.3]: https://github.com/yeasy/mdpress/compare/v0.6.2...v0.6.3
589593
[0.6.2]: https://github.com/yeasy/mdpress/compare/v0.6.1...v0.6.2
590594
[0.6.1]: https://github.com/yeasy/mdpress/compare/v0.6.0...v0.6.1

cmd/upgrade.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ func installNewVersion(ctx context.Context, release *gitHubRelease, newVersion s
314314
if err := os.Rename(tempPath, currentPath); err != nil {
315315
// Try to restore from backup on error.
316316
if restoreErr := os.Rename(backupPath, currentPath); restoreErr != nil {
317-
return fmt.Errorf("failed to install binary (%w), and restore failed (%v); backup at %s", err, restoreErr, backupPath)
317+
return fmt.Errorf("failed to install binary (%w), and restore failed (%w); backup at %s", err, restoreErr, backupPath)
318318
}
319319
return fmt.Errorf("failed to install binary (backup restored): %w", err)
320320
}

cmd/upgrade_test.go

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,21 @@ import (
66
"bytes"
77
"compress/gzip"
88
"context"
9+
"crypto/sha256"
10+
"encoding/hex"
911
"encoding/json"
12+
"errors"
1013
"fmt"
1114
"io"
1215
"net/http"
1316
"net/http/httptest"
17+
"net/url"
1418
"os"
1519
"path/filepath"
1620
"runtime"
21+
"strings"
1722
"testing"
23+
"time"
1824
)
1925

2026
// TestVersionComparison tests the version comparison logic.
@@ -893,3 +899,252 @@ func mustCreateZipArchive(t *testing.T, files map[string][]byte) []byte {
893899

894900
return buf.Bytes()
895901
}
902+
903+
// redirectTransport intercepts HTTP requests and routes them to a local test server
904+
// instead of the real host. This allows tests to use github.com URLs (passing
905+
// isGitHubDownloadURL validation) while actually hitting an httptest server.
906+
type redirectTransport struct {
907+
targetURL string // base URL of the httptest server
908+
}
909+
910+
func (rt *redirectTransport) RoundTrip(req *http.Request) (*http.Response, error) {
911+
// Rewrite the request URL to point at the test server, preserving the path.
912+
target, _ := url.Parse(rt.targetURL)
913+
req.URL.Scheme = target.Scheme
914+
req.URL.Host = target.Host
915+
return http.DefaultTransport.RoundTrip(req)
916+
}
917+
918+
// newTestRelease is a helper that builds a gitHubRelease with the given assets.
919+
func newTestRelease(assets ...struct {
920+
Name string
921+
URL string
922+
}) *gitHubRelease {
923+
release := &gitHubRelease{
924+
TagName: "v1.0.0",
925+
Assets: make([]struct {
926+
Name string `json:"name"`
927+
URL string `json:"browser_download_url"`
928+
}, len(assets)),
929+
}
930+
for i, a := range assets {
931+
release.Assets[i].Name = a.Name
932+
release.Assets[i].URL = a.URL
933+
}
934+
return release
935+
}
936+
937+
// TestVerifyChecksum tests the verifyChecksum function.
938+
func TestVerifyChecksum(t *testing.T) {
939+
// Compute a real SHA256 hash for test binary data.
940+
binaryData := []byte("test binary content for checksum verification")
941+
hash := sha256.Sum256(binaryData)
942+
correctHash := hex.EncodeToString(hash[:])
943+
wrongHash := "0000000000000000000000000000000000000000000000000000000000000000"
944+
945+
t.Run("no checksum file in release", func(t *testing.T) {
946+
release := newTestRelease() // empty assets
947+
err := verifyChecksum(context.Background(), release, "mdpress-linux-amd64", binaryData)
948+
if err == nil {
949+
t.Fatal("expected error for release with no checksum file")
950+
}
951+
if !strings.Contains(err.Error(), "no checksum file found") {
952+
t.Errorf("unexpected error: %v", err)
953+
}
954+
})
955+
956+
t.Run("no checksum file when assets exist but none match", func(t *testing.T) {
957+
release := newTestRelease(struct {
958+
Name string
959+
URL string
960+
}{"mdpress-linux-amd64", "https://github.com/yeasy/mdpress/releases/download/v1.0.0/mdpress-linux-amd64"})
961+
err := verifyChecksum(context.Background(), release, "mdpress-linux-amd64", binaryData)
962+
if err == nil {
963+
t.Fatal("expected error when no checksum asset in release")
964+
}
965+
if !strings.Contains(err.Error(), "no checksum file found") {
966+
t.Errorf("unexpected error: %v", err)
967+
}
968+
})
969+
970+
t.Run("checksum file with non-GitHub URL", func(t *testing.T) {
971+
release := newTestRelease(struct {
972+
Name string
973+
URL string
974+
}{"checksums.txt", "https://evil.example.com/checksums.txt"})
975+
err := verifyChecksum(context.Background(), release, "mdpress-linux-amd64", binaryData)
976+
if err == nil {
977+
t.Fatal("expected error for non-GitHub checksum URL")
978+
}
979+
if !strings.Contains(err.Error(), "unexpected host") {
980+
t.Errorf("unexpected error: %v", err)
981+
}
982+
})
983+
984+
t.Run("checksum file with http URL", func(t *testing.T) {
985+
release := newTestRelease(struct {
986+
Name string
987+
URL string
988+
}{"checksums.txt", "http://github.com/checksums.txt"})
989+
err := verifyChecksum(context.Background(), release, "mdpress-linux-amd64", binaryData)
990+
if err == nil {
991+
t.Fatal("expected error for http (non-https) checksum URL")
992+
}
993+
if !strings.Contains(err.Error(), "unexpected host") {
994+
t.Errorf("unexpected error: %v", err)
995+
}
996+
})
997+
998+
// For tests that require downloading the checksum file, set up an httptest server
999+
// and swap the upgradeHTTPClient to redirect github.com requests to it.
1000+
setupDownloadTest := func(t *testing.T, checksumContent string) *httptest.Server {
1001+
t.Helper()
1002+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1003+
w.Header().Set("Content-Type", "text/plain")
1004+
w.Write([]byte(checksumContent)) //nolint:errcheck
1005+
}))
1006+
t.Cleanup(server.Close)
1007+
1008+
origClient := upgradeHTTPClient
1009+
upgradeHTTPClient = &http.Client{
1010+
Timeout: 5 * time.Second,
1011+
Transport: &redirectTransport{targetURL: server.URL},
1012+
}
1013+
t.Cleanup(func() { upgradeHTTPClient = origClient })
1014+
1015+
return server
1016+
}
1017+
1018+
t.Run("valid checksum match", func(t *testing.T) {
1019+
checksumContent := fmt.Sprintf("%s mdpress-linux-amd64\n", correctHash)
1020+
setupDownloadTest(t, checksumContent)
1021+
1022+
release := newTestRelease(struct {
1023+
Name string
1024+
URL string
1025+
}{"checksums.txt", "https://github.com/yeasy/mdpress/releases/download/v1.0.0/checksums.txt"})
1026+
1027+
err := verifyChecksum(context.Background(), release, "mdpress-linux-amd64", binaryData)
1028+
if err != nil {
1029+
t.Errorf("expected nil error for valid checksum, got: %v", err)
1030+
}
1031+
})
1032+
1033+
t.Run("checksum mismatch", func(t *testing.T) {
1034+
checksumContent := fmt.Sprintf("%s mdpress-linux-amd64\n", wrongHash)
1035+
setupDownloadTest(t, checksumContent)
1036+
1037+
release := newTestRelease(struct {
1038+
Name string
1039+
URL string
1040+
}{"checksums.txt", "https://github.com/yeasy/mdpress/releases/download/v1.0.0/checksums.txt"})
1041+
1042+
err := verifyChecksum(context.Background(), release, "mdpress-linux-amd64", binaryData)
1043+
if err == nil {
1044+
t.Fatal("expected error for checksum mismatch")
1045+
}
1046+
if !errors.Is(err, errChecksumMismatch) {
1047+
t.Errorf("expected errChecksumMismatch, got: %v", err)
1048+
}
1049+
})
1050+
1051+
t.Run("no matching entry in checksum file", func(t *testing.T) {
1052+
checksumContent := fmt.Sprintf("%s mdpress-darwin-arm64\n", correctHash)
1053+
setupDownloadTest(t, checksumContent)
1054+
1055+
release := newTestRelease(struct {
1056+
Name string
1057+
URL string
1058+
}{"checksums.txt", "https://github.com/yeasy/mdpress/releases/download/v1.0.0/checksums.txt"})
1059+
1060+
err := verifyChecksum(context.Background(), release, "mdpress-linux-amd64", binaryData)
1061+
if err == nil {
1062+
t.Fatal("expected error when asset not found in checksum file")
1063+
}
1064+
if !errors.Is(err, errNoChecksumEntry) {
1065+
t.Errorf("expected errNoChecksumEntry, got: %v", err)
1066+
}
1067+
})
1068+
1069+
t.Run("case-insensitive hash comparison", func(t *testing.T) {
1070+
upperHash := strings.ToUpper(correctHash)
1071+
checksumContent := fmt.Sprintf("%s mdpress-linux-amd64\n", upperHash)
1072+
setupDownloadTest(t, checksumContent)
1073+
1074+
release := newTestRelease(struct {
1075+
Name string
1076+
URL string
1077+
}{"checksums.txt", "https://github.com/yeasy/mdpress/releases/download/v1.0.0/checksums.txt"})
1078+
1079+
err := verifyChecksum(context.Background(), release, "mdpress-linux-amd64", binaryData)
1080+
if err != nil {
1081+
t.Errorf("expected nil error for case-insensitive hash match, got: %v", err)
1082+
}
1083+
})
1084+
1085+
t.Run("sha256 file extension recognized", func(t *testing.T) {
1086+
checksumContent := fmt.Sprintf("%s mdpress-linux-amd64\n", correctHash)
1087+
setupDownloadTest(t, checksumContent)
1088+
1089+
release := newTestRelease(struct {
1090+
Name string
1091+
URL string
1092+
}{"checksums.sha256", "https://github.com/yeasy/mdpress/releases/download/v1.0.0/checksums.sha256"})
1093+
1094+
err := verifyChecksum(context.Background(), release, "mdpress-linux-amd64", binaryData)
1095+
if err != nil {
1096+
t.Errorf("expected nil error for .sha256 checksum file, got: %v", err)
1097+
}
1098+
})
1099+
1100+
t.Run("multiple entries in checksum file", func(t *testing.T) {
1101+
checksumContent := fmt.Sprintf(
1102+
"%s mdpress-darwin-arm64\n%s mdpress-linux-amd64\n%s mdpress-windows-amd64.exe\n",
1103+
wrongHash, correctHash, wrongHash,
1104+
)
1105+
setupDownloadTest(t, checksumContent)
1106+
1107+
release := newTestRelease(struct {
1108+
Name string
1109+
URL string
1110+
}{"checksums.txt", "https://github.com/yeasy/mdpress/releases/download/v1.0.0/checksums.txt"})
1111+
1112+
err := verifyChecksum(context.Background(), release, "mdpress-linux-amd64", binaryData)
1113+
if err != nil {
1114+
t.Errorf("expected nil error when correct entry exists among many, got: %v", err)
1115+
}
1116+
})
1117+
1118+
t.Run("single-space separator in checksum line", func(t *testing.T) {
1119+
// The parser uses strings.Fields, so single space should also work.
1120+
checksumContent := fmt.Sprintf("%s mdpress-linux-amd64\n", correctHash)
1121+
setupDownloadTest(t, checksumContent)
1122+
1123+
release := newTestRelease(struct {
1124+
Name string
1125+
URL string
1126+
}{"checksums.txt", "https://github.com/yeasy/mdpress/releases/download/v1.0.0/checksums.txt"})
1127+
1128+
err := verifyChecksum(context.Background(), release, "mdpress-linux-amd64", binaryData)
1129+
if err != nil {
1130+
t.Errorf("expected nil error for single-space separator, got: %v", err)
1131+
}
1132+
})
1133+
1134+
t.Run("empty checksum file", func(t *testing.T) {
1135+
setupDownloadTest(t, "")
1136+
1137+
release := newTestRelease(struct {
1138+
Name string
1139+
URL string
1140+
}{"checksums.txt", "https://github.com/yeasy/mdpress/releases/download/v1.0.0/checksums.txt"})
1141+
1142+
err := verifyChecksum(context.Background(), release, "mdpress-linux-amd64", binaryData)
1143+
if err == nil {
1144+
t.Fatal("expected error for empty checksum file")
1145+
}
1146+
if !errors.Is(err, errNoChecksumEntry) {
1147+
t.Errorf("expected errNoChecksumEntry, got: %v", err)
1148+
}
1149+
})
1150+
}

0 commit comments

Comments
 (0)