diff --git a/app/tts/engine/python/pack/catalog.go b/app/tts/engine/python/pack/catalog.go index 1f28d3d..b174662 100644 --- a/app/tts/engine/python/pack/catalog.go +++ b/app/tts/engine/python/pack/catalog.go @@ -21,6 +21,12 @@ type AvailablePack struct { BundleSize int64 `json:"bundleSize"` Installed bool `json:"installed"` TagName string `json:"tagName"` + // ExpectedSHA256 is the lowercase hex SHA-256 digest the downloaded bundle + // must match before it is extracted/executed. It is populated from the + // GitHub asset "digest" field ("sha256:") when the API exposes it, and + // is empty for older API responses or local packs that carry no trusted + // digest. + ExpectedSHA256 string `json:"expectedSha256"` } type githubRelease struct { @@ -33,6 +39,9 @@ type githubRelease struct { Name string `json:"name"` BrowserDownloadURL string `json:"browser_download_url"` Size int64 `json:"size"` + // Digest is provided by newer GitHub Releases API responses in the + // form "sha256:". Used to verify bundle integrity before install. + Digest string `json:"digest"` } `json:"assets"` } @@ -68,7 +77,7 @@ func FetchAvailable() ([]AvailablePack, error) { continue } - var bundleURL, metadataURL string + var bundleURL, metadataURL, bundleDigest string var bundleSize int64 for _, asset := range release.Assets { @@ -80,6 +89,7 @@ func FetchAvailable() ([]AvailablePack, error) { case strings.HasSuffix(lowerCaseAssetName, ".nstudio"): bundleURL = asset.BrowserDownloadURL bundleSize = asset.Size + bundleDigest = asset.Digest case strings.HasSuffix(lowerCaseAssetName, ".metadata.json"): metadataURL = asset.BrowserDownloadURL } @@ -95,17 +105,30 @@ func FetchAvailable() ([]AvailablePack, error) { } availablePacks = append(availablePacks, AvailablePack{ - Manifest: *manifest, - BundleURL: bundleURL, - BundleSize: bundleSize, - Installed: IsInstalled(manifest.ID), - TagName: release.TagName, + Manifest: *manifest, + BundleURL: bundleURL, + BundleSize: bundleSize, + Installed: IsInstalled(manifest.ID), + TagName: release.TagName, + ExpectedSHA256: parseSHA256Digest(bundleDigest), }) } return availablePacks, nil } +// parseSHA256Digest extracts the lowercase hex payload from a GitHub asset +// digest of the form "sha256:". Any other/empty value yields "", which the +// installer treats as "no trusted digest available". +func parseSHA256Digest(digest string) string { + digest = strings.TrimSpace(digest) + const prefix = "sha256:" + if !strings.HasPrefix(strings.ToLower(digest), prefix) { + return "" + } + return strings.ToLower(digest[len(prefix):]) +} + func fetchMetadata(metadataURL string) (*python.Manifest, error) { httpClient := &http.Client{Timeout: 20 * time.Second} metadataResponse, err := httpClient.Get(metadataURL) diff --git a/app/tts/engine/python/pack/install.go b/app/tts/engine/python/pack/install.go index 648c14b..dd94de4 100644 --- a/app/tts/engine/python/pack/install.go +++ b/app/tts/engine/python/pack/install.go @@ -2,7 +2,10 @@ package pack import ( "archive/zip" + "crypto/sha256" + "encoding/hex" "encoding/json" + "hash" "io" "net/http" "nstudio/app/common/eventManager" @@ -14,6 +17,8 @@ import ( "runtime" "strings" "time" + + "github.com/charmbracelet/log" ) const ( @@ -93,7 +98,7 @@ func Install(availablePack AvailablePack) error { progress := &progressReader{engineID: availablePack.Manifest.ID, totalBytes: availablePack.BundleSize} progress.emitProgressIfDue(true) - if err := downloadBundle(availablePack.BundleURL, archivePath, progress); err != nil { + if err := downloadBundle(availablePack.BundleURL, archivePath, availablePack.ExpectedSHA256, progress); err != nil { return response.Err("install: download: %w", err) } @@ -189,13 +194,22 @@ func isLocalBundlePath(bundleSource string) bool { return true } -func downloadBundle(bundleURL, destinationPath string, progress *progressReader) error { +func downloadBundle(bundleURL, destinationPath, expectedSHA256 string, progress *progressReader) error { if err := os.MkdirAll(filepath.Dir(destinationPath), 0o755); err != nil { return response.Err("%w", err) } if isLocalBundlePath(bundleURL) { - return copyLocalBundle(bundleURL, destinationPath, progress) + // Local file paths are trusted operator input, not a network fetch, so + // there is nothing to MITM. Still verify a digest if one was supplied. + return copyLocalBundle(bundleURL, destinationPath, expectedSHA256, progress) + } + + // Only https is acceptable for remote bundles. A plain http:// (or any other + // scheme) download can be transparently rewritten by a network attacker, and + // the bundle's interpreter is executed as native code after install. + if !strings.HasPrefix(strings.ToLower(bundleURL), "https://") { + return response.Err("refusing to download bundle over insecure URL (https required): %s", bundleURL) } httpClient := &http.Client{Timeout: 60 * time.Minute} @@ -214,10 +228,10 @@ func downloadBundle(bundleURL, destinationPath string, progress *progressReader) } progress.underlyingReader = downloadResponse.Body - return writeThroughTemporaryFile(progress, destinationPath) + return writeThroughTemporaryFile(progress, destinationPath, expectedSHA256) } -func copyLocalBundle(sourcePath, destinationPath string, progress *progressReader) error { +func copyLocalBundle(sourcePath, destinationPath, expectedSHA256 string, progress *progressReader) error { sourceFile, err := os.Open(sourcePath) if err != nil { return response.Err("open local bundle %s: %w", sourcePath, err) @@ -229,17 +243,32 @@ func copyLocalBundle(sourcePath, destinationPath string, progress *progressReade } progress.underlyingReader = sourceFile - return writeThroughTemporaryFile(progress, destinationPath) + return writeThroughTemporaryFile(progress, destinationPath, expectedSHA256) } -func writeThroughTemporaryFile(sourceReader io.Reader, destinationPath string) error { +// writeThroughTemporaryFile streams sourceReader to a ".part" temp file, then +// atomically renames it into place. When expectedSHA256 is non-empty the file's +// SHA-256 is computed while writing and verified before the rename; on mismatch +// the temp file is deleted and nothing is installed. When it is empty (no +// trusted digest available, e.g. an older GitHub API response or a local pack) a +// prominent warning is logged and the download proceeds unverified. +func writeThroughTemporaryFile(sourceReader io.Reader, destinationPath, expectedSHA256 string) error { temporaryFilePath := destinationPath + ".part" temporaryFile, err := os.Create(temporaryFilePath) if err != nil { return response.Err("%w", err) } - if _, err := io.Copy(temporaryFile, sourceReader); err != nil { + var digestWriter hash.Hash + writeTarget := io.Writer(temporaryFile) + if expectedSHA256 != "" { + digestWriter = sha256.New() + writeTarget = io.MultiWriter(temporaryFile, digestWriter) + } else { + log.Warnf("pack install: no trusted SHA-256 digest available for %s; installing UNVERIFIED bundle", filepath.Base(destinationPath)) + } + + if _, err := io.Copy(writeTarget, sourceReader); err != nil { temporaryFile.Close() _ = os.Remove(temporaryFilePath) return response.Err("%w", err) @@ -249,6 +278,14 @@ func writeThroughTemporaryFile(sourceReader io.Reader, destinationPath string) e return response.Err("%w", err) } + if digestWriter != nil { + actualSHA256 := hex.EncodeToString(digestWriter.Sum(nil)) + if !strings.EqualFold(actualSHA256, expectedSHA256) { + _ = os.Remove(temporaryFilePath) + return response.Err("bundle digest mismatch: expected sha256 %s, got %s", strings.ToLower(expectedSHA256), actualSHA256) + } + } + if err := os.Rename(temporaryFilePath, destinationPath); err != nil { _ = os.Remove(temporaryFilePath) return response.Err("%w", err) @@ -380,6 +417,15 @@ func validateManifest(manifest *python.Manifest, bundlePath string) error { if manifest.Runtime.Python == "" || manifest.Runtime.Entrypoint == "" { return response.Err("manifest runtime is missing python or entrypoint") } + // The interpreter/entrypoint are joined onto bundlePath and then executed, + // so a manifest that escapes the bundle (absolute path or "..") could run an + // arbitrary binary outside the extracted, verified bundle. + if err := validateBundleRelativePath(manifest.Runtime.Python); err != nil { + return response.Err("manifest runtime.python is unsafe: %w", err) + } + if err := validateBundleRelativePath(manifest.Runtime.Entrypoint); err != nil { + return response.Err("manifest runtime.entrypoint is unsafe: %w", err) + } interpreterPath := filepath.Join(bundlePath, filepath.FromSlash(manifest.Runtime.Python)) if _, err := os.Stat(interpreterPath); err != nil { return response.Err("bundle missing python at %s: %w", interpreterPath, err) @@ -390,3 +436,19 @@ func validateManifest(manifest *python.Manifest, bundlePath string) error { } return nil } + +// validateBundleRelativePath rejects manifest-supplied paths that are absolute +// or contain a ".." segment, so they cannot resolve outside the bundle when +// joined onto the bundle root. +func validateBundleRelativePath(rawPath string) error { + cleanedPath := filepath.FromSlash(rawPath) + if filepath.IsAbs(cleanedPath) || filepath.IsAbs(rawPath) { + return response.Err("absolute path not allowed: %q", rawPath) + } + for _, segment := range strings.Split(filepath.ToSlash(rawPath), "/") { + if segment == ".." { + return response.Err("path traversal not allowed: %q", rawPath) + } + } + return nil +} diff --git a/app/tts/engine/python/pack/install_test.go b/app/tts/engine/python/pack/install_test.go new file mode 100644 index 0000000..52878f9 --- /dev/null +++ b/app/tts/engine/python/pack/install_test.go @@ -0,0 +1,87 @@ +package pack + +import ( + "crypto/sha256" + "encoding/hex" + "os" + "path/filepath" + "strings" + "testing" +) + +func writeThroughForTest(t *testing.T, data []byte, expectedSHA256 string) error { + t.Helper() + destinationPath := filepath.Join(t.TempDir(), "bundle.nstudio") + return writeThroughTemporaryFile(strings.NewReader(string(data)), destinationPath, expectedSHA256) +} + +func TestWriteThroughTemporaryFile_DigestMatch(t *testing.T) { + data := []byte("hello bundle") + sum := sha256.Sum256(data) + if err := writeThroughForTest(t, data, hex.EncodeToString(sum[:])); err != nil { + t.Fatalf("expected matching digest to succeed, got: %v", err) + } +} + +func TestWriteThroughTemporaryFile_DigestMismatchAborts(t *testing.T) { + destinationPath := filepath.Join(t.TempDir(), "bundle.nstudio") + badDigest := strings.Repeat("0", 64) + err := writeThroughTemporaryFile(strings.NewReader("hello bundle"), destinationPath, badDigest) + if err == nil { + t.Fatal("expected digest mismatch error, got nil") + } + if _, statErr := os.Stat(destinationPath); !os.IsNotExist(statErr) { + t.Fatalf("expected no installed file on mismatch, stat err: %v", statErr) + } + if _, statErr := os.Stat(destinationPath + ".part"); !os.IsNotExist(statErr) { + t.Fatal("expected temp .part file to be removed on mismatch") + } +} + +func TestWriteThroughTemporaryFile_EmptyDigestProceeds(t *testing.T) { + if err := writeThroughForTest(t, []byte("unverified"), ""); err != nil { + t.Fatalf("expected empty digest to proceed unverified, got: %v", err) + } +} + +func TestDownloadBundle_RejectsInsecureURL(t *testing.T) { + destinationPath := filepath.Join(t.TempDir(), "bundle.nstudio") + progress := &progressReader{engineID: "test"} + err := downloadBundle("http://example.com/pack.nstudio", destinationPath, "", progress) + if err == nil { + t.Fatal("expected http:// URL to be rejected") + } + if !strings.Contains(err.Error(), "https required") { + t.Fatalf("expected https-required error, got: %v", err) + } +} + +func TestParseSHA256Digest(t *testing.T) { + cases := map[string]string{ + "sha256:ABCDEF": "abcdef", + "SHA256:abc123": "abc123", + "": "", + "md5:abc": "", + "abcdef": "", + } + for input, want := range cases { + if got := parseSHA256Digest(input); got != want { + t.Errorf("parseSHA256Digest(%q) = %q, want %q", input, got, want) + } + } +} + +func TestValidateBundleRelativePath(t *testing.T) { + valid := []string{"bin/python", "python3", "runtime/bin/python3.11"} + for _, p := range valid { + if err := validateBundleRelativePath(p); err != nil { + t.Errorf("expected %q to be valid, got: %v", p, err) + } + } + invalid := []string{"/usr/bin/python", "../python", "bin/../../python", "a/../../../etc/passwd"} + for _, p := range invalid { + if err := validateBundleRelativePath(p); err == nil { + t.Errorf("expected %q to be rejected", p) + } + } +}