diff --git a/internal/appherder/appimage.go b/internal/appherder/appimage.go index 5e84fea..9e1f38e 100644 --- a/internal/appherder/appimage.go +++ b/internal/appherder/appimage.go @@ -99,7 +99,7 @@ func fileSystemOffset(file io.ReaderAt) (int64, error) { return 0, errors.New("not an AppImage (missing ELF header)") } - // Byte 8-10: AppImage type. 1 = ISO 9660 (unsupported), 2 = appended filesystem. + // Type-1 AppImages use ISO 9660 instead of an appended filesystem. if header[8] == 'A' && header[9] == 'I' && header[10] == 1 { return 0, errors.New("type-1 AppImages are not supported") } @@ -109,7 +109,7 @@ func fileSystemOffset(file io.ReaderAt) (int64, error) { endian = binary.BigEndian } - // e_shoff + e_shnum * e_shentsize = end of the section header table. + // The payload starts after the section header table. switch header[4] { case 1: // 32-bit tableStart := int64(endian.Uint32(header[32:36])) diff --git a/internal/appherder/appimage_test.go b/internal/appherder/appimage_test.go index 13c7ece..93ac172 100644 --- a/internal/appherder/appimage_test.go +++ b/internal/appherder/appimage_test.go @@ -350,7 +350,7 @@ func TestSaveToVersionsPrunesOldestVersions(t *testing.T) { t.Fatal(err) } - // Seed 3 existing versions with distinct mtimes. + // Seed existing versions with distinct mtimes. t1 := time.Now().Add(-3 * time.Hour) t2 := time.Now().Add(-2 * time.Hour) t3 := time.Now().Add(-1 * time.Hour) @@ -372,7 +372,7 @@ func TestSaveToVersionsPrunesOldestVersions(t *testing.T) { t.Fatal(err) } - // Saving a 4th version should prune v1 (oldest) + // Saving another version prunes v1, the oldest. if err := a.saveToVersions(current, "foo"); err != nil { t.Fatal(err) } diff --git a/internal/appherder/http.go b/internal/appherder/http.go index ff7f474..285ed1a 100644 --- a/internal/appherder/http.go +++ b/internal/appherder/http.go @@ -35,8 +35,7 @@ const ( downloadIdleTimeout = 60 * time.Second ) -// idleTimeoutReader cancels via cancel() when a single Read stalls longer than -// timeout, guarding a download against a connection that goes quiet. +// idleTimeoutReader cancels stalled downloads without limiting total duration. type idleTimeoutReader struct { reader io.Reader timer *time.Timer @@ -56,9 +55,8 @@ func (t *idleTimeoutReader) Read(buf []byte) (int, error) { return bytesRead, err } -// httpGetOK sends a GET request and returns the response when the server -// returns 200, closing the body and returning an error otherwise. customize -// may set headers before the request is sent. The caller closes resp.Body. +// httpGetOK returns a 200 response, closing the body before returning errors. +// customize may add request headers. The caller closes resp.Body on success. func httpGetOK(ctx context.Context, url, desc string, customize func(*http.Request)) (*http.Response, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { diff --git a/internal/appherder/install.go b/internal/appherder/install.go index 12683f0..c33205d 100644 --- a/internal/appherder/install.go +++ b/internal/appherder/install.go @@ -20,9 +20,8 @@ func (a App) install(ctx context.Context, appimage string, want expectedChecksum return "", fmt.Errorf("resolve AppImage path %q: %w", appimage, err) } - // Verify before openAppImage: the DwarFS fallback executes the AppImage to - // extract it, so bad images must be refused first. Pinned-key check is - // deferred until the app name is known from the desktop file inside. + // Verify before openAppImage, because DwarFS extraction executes the AppImage. + // Defer pinned-key checks until the desktop file reveals the app name. fingerprint, err := verifyAppImage(appimage, "", want) if err != nil { return "", err @@ -49,7 +48,6 @@ func (a App) install(ctx context.Context, appimage string, want expectedChecksum icon := resolveIcon(fsys) appName = deriveAppName(desktop, desktopName, appimage) - // Pinned-key check deferred from above; app name now known. pinned := a.pinnedSigningKey(appName) if pinned != "" { if fingerprint == "" { @@ -72,8 +70,7 @@ func (a App) install(ctx context.Context, appimage string, want expectedChecksum iconPath = preparedIcon.path } - // No desktop file inside the AppImage: synthesize a terminal launcher so - // CLI apps still get a menu entry and are tracked by managedApps. + // CLI AppImages often omit desktop files; synthesize one so Sync can track them. if desktop == nil { desktop = desktopfile.Parse([]byte(fmt.Sprintf( "[Desktop Entry]\nType=Application\nName=%s\nTerminal=true\n", @@ -81,7 +78,7 @@ func (a App) install(ctx context.Context, appimage string, want expectedChecksum ))) } - // Patch in memory before any filesystem writes so a failure here installs nothing. + // Patch before filesystem writes so a failure installs nothing. if err := a.patchDesktopFile(desktop, appName, iconPath); err != nil { return "", err } @@ -89,7 +86,6 @@ func (a App) install(ctx context.Context, appimage string, want expectedChecksum desktop.Set(desktopEntrySection, desktopSigningKey, pin) } - // Roll back written files on a later failure rather than leaving a half-installed app. var installed []string rollback := func() { for _, path := range installed { @@ -113,9 +109,7 @@ func (a App) install(ctx context.Context, appimage string, want expectedChecksum } installed = append(installed, dest) - // Materialize the AppImage last: when the source is already in ~/AppImages - // it gets moved, so an earlier failure must not roll back over the user's - // file. + // Install the AppImage last so earlier failures never remove the user's source file. if _, err := a.installAppImage(appimage, appName); err != nil { rollback() return "", err diff --git a/internal/appherder/rollback_test.go b/internal/appherder/rollback_test.go index ffc01ed..215ba7c 100644 --- a/internal/appherder/rollback_test.go +++ b/internal/appherder/rollback_test.go @@ -103,13 +103,13 @@ func TestRollbackSavesCurrentVersionBeforeRestoring(t *testing.T) { t.Fatal(err) } - // Current should now be "old" + // The restored version becomes current. got, _ := os.ReadFile(current) if string(got) != "old" { t.Fatalf("current = %q, want old", string(got)) } - // Previous current should be saved; the restored version moves to active. + // The previous current file is saved as the only remaining version. entries, _ := os.ReadDir(versionsDir) if len(entries) != 1 { t.Fatalf("expected 1 saved version (pre-rollback current), got %d", len(entries)) diff --git a/internal/appherder/signature.go b/internal/appherder/signature.go index bb2f41a..512e60d 100644 --- a/internal/appherder/signature.go +++ b/internal/appherder/signature.go @@ -42,7 +42,7 @@ type expectedChecksum struct { func (c expectedChecksum) set() bool { return c.hex != "" } -// matches reports whether the bytes fed to c.hasher hash to the advertised value. +// matches reports whether c.hasher matches the advertised checksum. func (c expectedChecksum) matches() bool { return strings.EqualFold(hex.EncodeToString(c.hasher.Sum(nil)), c.hex) } @@ -62,8 +62,7 @@ func sectionData(f *elf.File, name string) (data []byte, span byteRange, ok bool return bytes.TrimRight(data, "\x00"), byteRange{int64(section.Offset), int64(section.Size)}, true, nil } -// readSignatureSections returns the .sha256_sig and .sig_key contents and the -// byte ranges they occupy, which the signing digest zeroes. +// readSignatureSections returns the signature sections and their file spans. func readSignatureSections(file string) (sig, key []byte, zero []byteRange, err error) { elfFile, err := elf.Open(file) if err != nil { @@ -149,8 +148,6 @@ func verifyAppImage(file, pinned string, want expectedChecksum) (fingerprint str } signed := len(bytes.TrimSpace(sig)) > 0 - // Single read pass: hash only what we check. rawHash takes the unmodified - // bytes for the checksum, signHash the section-zeroed bytes for the digest. var rawHash, signHash hash.Hash if want.set() { rawHash = want.hasher diff --git a/internal/appherder/source.go b/internal/appherder/source.go index 332eafc..4af4e6c 100644 --- a/internal/appherder/source.go +++ b/internal/appherder/source.go @@ -22,8 +22,8 @@ import ( type Release struct { Version string // human label, e.g. a release tag URL string // download URL for the AppImage - SHA256 string // hex sha256 of the asset, "" when unavailable - SHA1 string // hex sha1 (zsync's hash), "" when unavailable + SHA256 string // hex-encoded SHA-256, "" when unavailable + SHA1 string // hex-encoded zsync SHA-1, "" when unavailable Size int64 // content length, 0 when unavailable ModTime time.Time // server Last-Modified, for checksumless sources } @@ -55,8 +55,6 @@ func (r Release) localMatches(file string) (bool, error) { if err != nil { return false, err } - // No checksum: current if the install is at least as new as the server's - // copy, else fall back to size. if !r.ModTime.IsZero() { return !info.ModTime().Before(r.ModTime), nil } @@ -148,13 +146,12 @@ func sourceFromELF(file string) (Source, error) { return parseUpdateInfo(info) } -// parseUpdateInfo turns an AppImage update-info string (the "type|a|b|..." form) +// parseUpdateInfo turns an AppImage update-info string ("type|arg|arg|...") // into a concrete source. func parseUpdateInfo(info string) (Source, error) { fields := strings.Split(info, "|") switch fields[0] { case "gh-releases-zsync", "gh-releases-direct": - // gh-releases-zsync|owner|repo|tag|pattern.zsync if len(fields) != 5 { return nil, fmt.Errorf("malformed GitHub update info %q", info) } @@ -165,7 +162,6 @@ func parseUpdateInfo(info string) (Source, error) { pattern: strings.TrimSuffix(fields[4], ".zsync"), }, nil case "gl-releases-zsync", "gl-releases-direct": - // gl-releases-zsync|host|project|tag|pattern.zsync (our convention) if len(fields) != 5 { return nil, fmt.Errorf("malformed GitLab update info %q", info) } @@ -176,13 +172,11 @@ func parseUpdateInfo(info string) (Source, error) { pattern: strings.TrimSuffix(fields[4], ".zsync"), }, nil case "zsync": - // zsync|https://host/path/App-latest.AppImage.zsync if len(fields) != 2 || fields[1] == "" { return nil, fmt.Errorf("malformed zsync update info %q", info) } return zsyncURLSource{url: fields[1]}, nil case "static": - // static|https://host/App-latest.AppImage (our convention) if len(fields) != 2 || fields[1] == "" { return nil, fmt.Errorf("malformed static update info %q", info) } @@ -219,7 +213,8 @@ func matchByName[T any](items []T, pattern string, name func(T) string, kind str return matches[0], nil } -const apiResponseLimit = 4 * 1024 * 1024 // 4 MiB; real API responses are a few KB +// apiResponseLimit protects JSON and zsync parsers from unexpectedly large responses. +const apiResponseLimit = 4 * 1024 * 1024 // decodeJSON decodes a JSON value from r, wrapping any error with desc. func decodeJSON[T any](r io.Reader, desc string) (T, error) { diff --git a/internal/appherder/source_gitlab.go b/internal/appherder/source_gitlab.go index 0ce0d11..b5643fc 100644 --- a/internal/appherder/source_gitlab.go +++ b/internal/appherder/source_gitlab.go @@ -36,7 +36,7 @@ func (s gitlabReleaseSource) Latest(ctx context.Context) (Release, error) { if base == "" { base = "https://" + s.host } - // The project path goes in one segment, so its slashes must be encoded. + // Encode slashes because GitLab expects the project path in one segment. endpoint := fmt.Sprintf("%s/api/v4/projects/%s/releases/", base, url.PathEscape(s.project)) if s.tag == "" || s.tag == "latest" { endpoint += "permalink/latest" @@ -69,8 +69,7 @@ func (s gitlabReleaseSource) Latest(ctx context.Context) (Release, error) { } out := Release{Version: rel.TagName, URL: linkURL(asset)} - // No digest from the API: take the checksum from the sibling .zsync asset - // when present, else comparison falls back to size. + // GitLab release links have no digest; sibling .zsync assets carry SHA-1. if zsyncLink, ok := findLink(rel.Assets.Links, asset.Name+".zsync"); ok { header, err := fetchZsyncHeader(ctx, linkURL(zsyncLink)) if err != nil { @@ -93,7 +92,7 @@ func findLink(links []glLink, name string) (glLink, bool) { return glLink{}, false } -// linkURL prefers the permalinked direct_asset_url, falling back to the raw url. +// linkURL prefers GitLab's permalinked asset URL. func linkURL(link glLink) string { if link.DirectAssetURL != "" { return link.DirectAssetURL diff --git a/internal/appherder/source_static.go b/internal/appherder/source_static.go index 0f89e28..30a0395 100644 --- a/internal/appherder/source_static.go +++ b/internal/appherder/source_static.go @@ -8,8 +8,7 @@ import ( ) // staticURLSource tracks a fixed URL that always serves the latest AppImage. -// There's no version or checksum, so freshness leans on the server's -// Last-Modified (vs. the installed file's mtime), then Content-Length. +// Without a version or checksum, freshness uses Last-Modified, then Content-Length. type staticURLSource struct { url string } @@ -39,8 +38,8 @@ func (s staticURLSource) Latest(ctx context.Context) (Release, error) { return rel, nil } -// probe reads the URL's headers via HEAD, falling back to GET for servers that -// reject it. The caller closes the body; we never read it. +// probe reads headers with HEAD, falling back to GET for servers that reject it. +// The caller closes the response body. func (s staticURLSource) probe(ctx context.Context) (*http.Response, error) { for _, method := range []string{http.MethodHead, http.MethodGet} { req, err := http.NewRequestWithContext(ctx, method, s.url, nil) diff --git a/internal/appherder/source_zsync.go b/internal/appherder/source_zsync.go index e216f5f..dd2e2e2 100644 --- a/internal/appherder/source_zsync.go +++ b/internal/appherder/source_zsync.go @@ -42,8 +42,7 @@ func (s zsyncURLSource) Latest(ctx context.Context) (Release, error) { return rel, nil } -// fetchZsyncHeader downloads a .zsync control file and returns its parsed -// header. Reused wherever a checksum can be read from a sibling .zsync asset. +// fetchZsyncHeader returns the parsed header from a .zsync control file. func fetchZsyncHeader(ctx context.Context, zsyncURL string) (map[string]string, error) { ctx, cancel := context.WithTimeout(ctx, apiTimeout) defer cancel() @@ -62,15 +61,14 @@ func fetchZsyncHeader(ctx context.Context, zsyncURL string) (map[string]string, return header, nil } -// parseZsyncHeader reads a .zsync file's text header: "Key: Value" lines ending -// at the blank line that separates them from the binary checksum block. Keys -// are lowercased. It stops at the blank line so the body isn't consumed. +// parseZsyncHeader reads "Key: Value" lines until the binary checksum block. +// Header keys are lowercased. func parseZsyncHeader(reader io.Reader) (map[string]string, error) { header := make(map[string]string) scanner := bufio.NewScanner(reader) for scanner.Scan() { line := scanner.Text() - if line == "" { // header ends; binary data follows + if line == "" { break } key, value, ok := strings.Cut(line, ":")