diff --git a/cmd/discover.go b/cmd/discover.go index 8614e7b..1d416a2 100644 --- a/cmd/discover.go +++ b/cmd/discover.go @@ -54,7 +54,7 @@ var discoverInstallsCmd = &cobra.Command{ if inst.IsInjected() { bdStatus = "yes" } - fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\n", ch.Name(), inst.Version, typeLabel, bdStatus, inst.CorePath) + fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\n", ch.Name(), inst.Version, typeLabel, bdStatus, inst.ResourcesPath) } } diff --git a/cmd/install.go b/cmd/install.go index cf88141..f7539ce 100644 --- a/cmd/install.go +++ b/cmd/install.go @@ -2,7 +2,9 @@ package cmd import ( "fmt" + "os" "path" + "strings" "github.com/spf13/cobra" @@ -14,6 +16,7 @@ import ( func init() { installCmd.Flags().StringP("path", "p", "", "Path to a Discord installation") installCmd.Flags().StringP("channel", "c", "stable", "Discord release channel (stable|ptb|canary)") + installCmd.Flags().Bool("dev", false, "Use the development build of BetterDiscord") rootCmd.AddCommand(installCmd) } @@ -23,6 +26,7 @@ var installCmd = &cobra.Command{ Short: "Installs BetterDiscord to your Discord", Long: "Install BetterDiscord by specifying either --path to a Discord install or --channel to auto-detect (default: stable).", RunE: func(cmd *cobra.Command, args []string) error { + // Handle path and channel flags, ensuring they are mutually exclusive pathFlag, _ := cmd.Flags().GetString("path") channelFlag, _ := cmd.Flags().GetString("channel") @@ -33,6 +37,14 @@ var installCmd = &cobra.Command{ return fmt.Errorf("--path and --channel are mutually exclusive") } + // Check if the --dev flag is set or if the BDCLI_DEV_BUILD environment variable is enabled + useDevBuild := false + devFlag, _ := cmd.Flags().GetBool("dev") + if devFlag || isDevBuildEnvEnabled() { + useDevBuild = true + output.Println("โš ๏ธ Using development build of BetterDiscord") + } + var install *discord.DiscordInstall if pathProvided { @@ -42,18 +54,18 @@ var installCmd = &cobra.Command{ } } else { channel := models.ParseChannel(channelFlag) - corePath := discord.GetSuggestedPath(channel) - install = discord.ResolvePath(corePath) + resourcesPath := discord.GetSuggestedPath(channel) + install = discord.ResolvePath(resourcesPath) if install == nil { return fmt.Errorf("could not find a valid %s installation to install to", channelFlag) } } - if err := install.InstallBD(); err != nil { + if err := install.InstallBD(models.InstallOptions{RestartDiscord: true, UseDevBuild: useDevBuild}); err != nil { return fmt.Errorf("installation failed: %w", err) } - output.Printf("โœ… BetterDiscord installed to %s\n", path.Dir(install.CorePath)) + output.Printf("โœ… BetterDiscord installed to %s\n", path.Dir(install.ResourcesPath)) output.Blank() output.Printf("๐Ÿ“‹ Installation Summary:\n") output.Blank() @@ -67,11 +79,24 @@ var installCmd = &cobra.Command{ } return "native" }()) - output.Printf(" Core Path: %s\n", path.Dir(install.CorePath)) + output.Printf(" Resources Path: %s\n", path.Dir(install.ResourcesPath)) output.Blank() - bdinstall := install.GetBetterDiscordInstall() + bdinstall, err := install.GetBetterDiscordInstall() + if err != nil { + output.Printf("failed to get BetterDiscord install info: %s\n", err.Error()) + return nil + } + if bdinstall == nil { + output.Printf("BetterDiscord install info is nil\n") + return nil + } bdinstall.LogBuildinfo() return nil }, } + +func isDevBuildEnvEnabled() bool { + value := strings.TrimSpace(strings.ToLower(os.Getenv("BDCLI_DEV_BUILD"))) + return value == "1" || value == "true" || value == "yes" +} diff --git a/cmd/uninstall.go b/cmd/uninstall.go index ce667ad..ba30202 100644 --- a/cmd/uninstall.go +++ b/cmd/uninstall.go @@ -90,11 +90,11 @@ var uninstallCmd = &cobra.Command{ } } - if err := install.UninstallBD(); err != nil { + if err := install.UninstallBD(models.UninstallOptions{FullUninstall: false, RestartDiscord: true}); err != nil { return fmt.Errorf("uninstallation failed: %w", err) } - output.Printf("โœ… BetterDiscord uninstalled from %s\n", path.Dir(install.CorePath)) + output.Printf("โœ… BetterDiscord uninstalled from %s\n", path.Dir(install.ResourcesPath)) return nil }, } @@ -105,7 +105,7 @@ func getAllInstalls() []*discord.DiscordInstall { seen := map[string]bool{} var installs []*discord.DiscordInstall - // Flatten the map of installs and filter out duplicates based on CorePath + // Flatten the map of installs and filter out duplicates based on ResourcesPath // Honestly, probably should have just returned a flat list from GetAllInstalls in the first place, but whatever // And also the chance of actually having duplicates is pretty much zero, but this is just in case // If you are reading this and you do have duplicates, please tell me because that would be very interesting and I would like to know how that happened @@ -115,10 +115,10 @@ func getAllInstalls() []*discord.DiscordInstall { if inst == nil { continue } - if seen[inst.CorePath] { + if seen[inst.ResourcesPath] { continue } - seen[inst.CorePath] = true + seen[inst.ResourcesPath] = true installs = append(installs, inst) } } @@ -129,11 +129,11 @@ func getAllInstalls() []*discord.DiscordInstall { func uninstallAll(installs []*discord.DiscordInstall) error { var firstErr error for _, inst := range installs { - if err := inst.UninstallBD(); err != nil { + if err := inst.UninstallBD(models.UninstallOptions{FullUninstall: false, RestartDiscord: true}); err != nil { if firstErr == nil { firstErr = err } - output.Printf("โŒ Failed to uninstall from %s\n", path.Dir(inst.CorePath)) + output.Printf("โŒ Failed to uninstall from %s\n", path.Dir(inst.ResourcesPath)) output.Printf(" %s\n", err.Error()) } } @@ -148,8 +148,8 @@ func removeAllBetterDiscord(installs []*discord.DiscordInstall) error { // so we need to filter them out to avoid trying to delete the same // folder multiple times for _, inst := range installs { - bd := inst.GetBetterDiscordInstall() - if bd == nil { + bd, err := inst.GetBetterDiscordInstall() + if err != nil || bd == nil { continue } roots[bd.Root()] = bd diff --git a/cmd/update.go b/cmd/update.go index 94b0d2c..eca9f90 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -16,6 +16,11 @@ func init() { rootCmd.AddCommand(updateCmd) } +// This currently only checks for updates to the BetterDiscord loader (betterdiscord.asar). +// In the future, we may also want to check for updates to the CLI itself. +// This also only checks for updates to the stable release as the check is cheap (tag name) +// the canary version is a rolling release and is not versioned, so it is not as easily +// possible to check for updates to it. var updateCmd = &cobra.Command{ Use: "update", Short: "Update BetterDiscord to the latest version", @@ -62,7 +67,7 @@ var updateCmd = &cobra.Command{ // Download the latest version output.Println("๐Ÿ“ฅ Downloading update...") - if err := bdinstall.Download(); err != nil { + if err := bdinstall.Download(false); err != nil { return fmt.Errorf("failed to download update: %w", err) } diff --git a/internal/betterdiscord/download.go b/internal/betterdiscord/download.go index 3704307..4582309 100644 --- a/internal/betterdiscord/download.go +++ b/internal/betterdiscord/download.go @@ -8,13 +8,33 @@ import ( "github.com/betterdiscord/cli/internal/utils" ) -func (i *BDInstall) download() error { +// Endpoints for fetching the BetterDiscord asar. Declared as package vars so +// tests can point them at a local httptest server. +var ( + websiteAsarURL = "https://betterdiscord.app/Download/betterdiscord.asar" + githubLatestReleaseURL = "https://api.github.com/repos/BetterDiscord/BetterDiscord/releases/latest" + // githubCanaryReleaseURL is the rolling pre-release tagged "canary" (rebuilt + // on every merge to the development branch). It is GitHub-only โ€” the website + // has no mirror โ€” so the dev-build path fetches it by tag rather than via the + // "latest" endpoint, which excludes pre-releases by design. + githubCanaryReleaseURL = "https://api.github.com/repos/BetterDiscord/BetterDiscord/releases/tags/canary" +) + +func (i *BDInstall) download(useDevBuild bool) error { if i.hasDownloaded { output.Printf("โœ… Already downloaded to %s\n", i.asar) return nil } - resp, err := utils.DownloadFile("https://betterdiscord.app/Download/betterdiscord.asar", i.asar) + // The development build lives only on GitHub, so skip the website leg and go + // straight to the canary release. A failure here must NOT fall back to the + // stable asar: a developer who asked for the dev build silently receiving + // stable is a confusing, near-undetectable footgun. + if useDevBuild { + return i.downloadFromGitHubRelease(githubCanaryReleaseURL, "GitHub (development build)") + } + + resp, err := utils.DownloadFile(websiteAsarURL, i.asar) if err == nil { version := resp.Header.Get("x-bd-version") if version == "" { @@ -31,10 +51,16 @@ func (i *BDInstall) download() error { output.Println("๐Ÿ” Falling back to GitHub...") } - // Get download URL from GitHub API - apiData, err := utils.DownloadJSON[models.GitHubRelease]("https://api.github.com/repos/BetterDiscord/BetterDiscord/releases/latest") + return i.downloadFromGitHubRelease(githubLatestReleaseURL, "GitHub") +} + +// downloadFromGitHubRelease fetches the release metadata at apiURL, locates the +// betterdiscord.asar asset, and downloads it into the BD folder. sourceLabel is +// used only for logging (e.g. "GitHub" or "GitHub (development build)"). +func (i *BDInstall) downloadFromGitHubRelease(apiURL, sourceLabel string) error { + apiData, err := utils.DownloadJSON[models.GitHubRelease](apiURL) if err != nil { - output.Println("โŒ Failed to get asset url from GitHub") + output.Printf("โŒ Failed to get asset url from %s\n", sourceLabel) output.Printf("โŒ %s\n", err.Error()) return err } @@ -48,8 +74,8 @@ func (i *BDInstall) download() error { } if index == -1 { - output.Println("โŒ Failed to find the BetterDiscord asar on GitHub") - return fmt.Errorf("failed to find betterdiscord.asar asset in GitHub release") + output.Printf("โŒ Failed to find the BetterDiscord asar on %s\n", sourceLabel) + return fmt.Errorf("failed to find betterdiscord.asar asset in %s release", sourceLabel) } var downloadUrl = apiData.Assets[index].URL @@ -62,15 +88,18 @@ func (i *BDInstall) download() error { // Download asar into the BD folder _, err = utils.DownloadFile(downloadUrl, i.asar) if err != nil { - output.Println("โŒ Failed to download BetterDiscord from GitHub") + output.Printf("โŒ Failed to download BetterDiscord from %s\n", sourceLabel) output.Printf("โŒ %s\n", err.Error()) return err } - if version == "" { - output.Println("โœ… Downloaded BetterDiscord from GitHub") - } else { - output.Printf("โœ… Downloaded BetterDiscord version %s from GitHub\n", output.FormatVersion(version)) + switch version { + case "": + output.Printf("โœ… Downloaded BetterDiscord from %s\n", sourceLabel) + case "canary": + output.Printf("โœ… Downloaded BetterDiscord development build from %s\n", sourceLabel) + default: + output.Printf("โœ… Downloaded BetterDiscord version %s from %s\n", output.FormatVersion(version), sourceLabel) } i.hasDownloaded = true diff --git a/internal/betterdiscord/download_test.go b/internal/betterdiscord/download_test.go new file mode 100644 index 0000000..60fff54 --- /dev/null +++ b/internal/betterdiscord/download_test.go @@ -0,0 +1,212 @@ +package betterdiscord + +import ( + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" +) + +// withURLs temporarily overrides the download endpoints for a test and restores +// them on cleanup. +func withURLs(t *testing.T, website, github string) { + t.Helper() + origWebsite, origGithub := websiteAsarURL, githubLatestReleaseURL + websiteAsarURL = website + githubLatestReleaseURL = github + t.Cleanup(func() { + websiteAsarURL = origWebsite + githubLatestReleaseURL = origGithub + }) +} + +// withCanaryURL temporarily overrides the canary (development build) endpoint for +// a test and restores it on cleanup. +func withCanaryURL(t *testing.T, canary string) { + t.Helper() + orig := githubCanaryReleaseURL + githubCanaryReleaseURL = canary + t.Cleanup(func() { + githubCanaryReleaseURL = orig + }) +} + +func newBDInstallWithDataDir(t *testing.T) *BDInstall { + t.Helper() + install := New(filepath.Join(t.TempDir(), "BetterDiscord")) + if err := os.MkdirAll(install.Data(), 0o755); err != nil { + t.Fatalf("failed to create data dir: %v", err) + } + return install +} + +func assertFileContents(t *testing.T, path, want string) { + t.Helper() + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("failed to read %s: %v", path, err) + } + if string(got) != want { + t.Errorf("contents of %s = %q, expected %q", path, string(got), want) + } +} + +func TestDownload_FromWebsite(t *testing.T) { + const body = "asar-from-website" + website := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("x-bd-version", "1.2.3") + fmt.Fprint(w, body) //nolint this is a test file + })) + defer website.Close() + + github := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("GitHub fallback should not be called when the website succeeds") + http.Error(w, "unexpected", http.StatusInternalServerError) + })) + defer github.Close() + + withURLs(t, website.URL, github.URL) + + install := newBDInstallWithDataDir(t) + if err := install.download(false); err != nil { + t.Fatalf("download() failed: %v", err) + } + if !install.HasDownloaded() { + t.Error("expected HasDownloaded() to be true") + } + assertFileContents(t, install.Asar(), body) +} + +func TestDownload_FallsBackToGitHub(t *testing.T) { + const body = "asar-from-github" + asset := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, body) //nolint this is a test file + })) + defer asset.Close() + + // Website fails, so download() should fall back to the GitHub release. + website := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "down", http.StatusInternalServerError) + })) + defer website.Close() + + github := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, `{"tag_name":"v9.9.9","assets":[{"name":"betterdiscord.asar","url":%q}]}`, asset.URL) + })) + defer github.Close() + + withURLs(t, website.URL, github.URL) + + install := newBDInstallWithDataDir(t) + if err := install.download(false); err != nil { + t.Fatalf("download() failed: %v", err) + } + if !install.HasDownloaded() { + t.Error("expected HasDownloaded() to be true after GitHub fallback") + } + assertFileContents(t, install.Asar(), body) +} + +func TestDownload_GitHubMissingAsset(t *testing.T) { + website := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "down", http.StatusInternalServerError) + })) + defer website.Close() + + github := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `{"tag_name":"v9.9.9","assets":[{"name":"something-else.zip","url":"http://example.invalid"}]}`) //nolint this is a test file + })) + defer github.Close() + + withURLs(t, website.URL, github.URL) + + install := newBDInstallWithDataDir(t) + if err := install.download(false); err == nil { + t.Fatal("expected an error when the betterdiscord.asar asset is missing") + } +} + +func TestDownload_DevBuildUsesCanaryAndSkipsWebsite(t *testing.T) { + const body = "asar-from-canary" + asset := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, body) //nolint this is a test file + })) + defer asset.Close() + + // Neither the website nor the "latest" endpoint should be touched for a dev build. + website := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("website should not be called for the development build") + http.Error(w, "unexpected", http.StatusInternalServerError) + })) + defer website.Close() + + github := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("the GitHub latest endpoint should not be called for the development build") + http.Error(w, "unexpected", http.StatusInternalServerError) + })) + defer github.Close() + + canary := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, `{"tag_name":"canary","assets":[{"name":"betterdiscord.asar","url":%q}]}`, asset.URL) + })) + defer canary.Close() + + withURLs(t, website.URL, github.URL) + withCanaryURL(t, canary.URL) + + install := newBDInstallWithDataDir(t) + if err := install.download(true); err != nil { + t.Fatalf("download(true) failed: %v", err) + } + if !install.HasDownloaded() { + t.Error("expected HasDownloaded() to be true after canary download") + } + assertFileContents(t, install.Asar(), body) +} + +func TestDownload_DevBuildHardFailsWithoutStableFallback(t *testing.T) { + // The canary release is unreachable. The dev build must fail rather than + // silently falling back to the website or the stable GitHub release. + website := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("website should not be called as a fallback for the development build") + http.Error(w, "unexpected", http.StatusInternalServerError) + })) + defer website.Close() + + github := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("the GitHub latest endpoint should not be called as a fallback for the development build") + http.Error(w, "unexpected", http.StatusInternalServerError) + })) + defer github.Close() + + canary := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "down", http.StatusInternalServerError) + })) + defer canary.Close() + + withURLs(t, website.URL, github.URL) + withCanaryURL(t, canary.URL) + + install := newBDInstallWithDataDir(t) + if err := install.download(true); err == nil { + t.Fatal("expected an error when the canary release is unreachable") + } + if install.HasDownloaded() { + t.Error("expected HasDownloaded() to remain false after a failed dev build download") + } +} + +func TestDownload_SkipsWhenAlreadyDownloaded(t *testing.T) { + install := newBDInstallWithDataDir(t) + install.hasDownloaded = true + + // Point the endpoints at a non-routable address to prove the network is + // never touched when the asar is already downloaded. + withURLs(t, "http://127.0.0.1:0", "http://127.0.0.1:0") + + if err := install.download(false); err != nil { + t.Fatalf("download() should be a no-op when already downloaded: %v", err) + } +} diff --git a/internal/betterdiscord/install.go b/internal/betterdiscord/install.go index 7bce735..bec02ec 100644 --- a/internal/betterdiscord/install.go +++ b/internal/betterdiscord/install.go @@ -51,9 +51,10 @@ func (i *BDInstall) HasDownloaded() bool { return i.hasDownloaded } -// Download downloads the BetterDiscord asar file -func (i *BDInstall) Download() error { - return i.download() +// Download downloads the BetterDiscord asar file. When useDevBuild is true it +// pulls the rolling "canary" pre-release from GitHub instead of the stable asar. +func (i *BDInstall) Download(useDevBuild bool) error { + return i.download(useDevBuild) } // Prepare creates all necessary directories for BetterDiscord diff --git a/internal/betterdiscord/meta_test.go b/internal/betterdiscord/meta_test.go index 4041591..47a442c 100644 --- a/internal/betterdiscord/meta_test.go +++ b/internal/betterdiscord/meta_test.go @@ -253,7 +253,7 @@ func BenchmarkParseJSDoc(b *testing.B) { * @version 1.0.0 */ ` - for i := 0; i < b.N; i++ { + for b.Loop() { parseJSDoc(input) } } diff --git a/internal/discord/assets/app_index.js b/internal/discord/assets/app_index.js new file mode 100644 index 0000000..ed9f932 --- /dev/null +++ b/internal/discord/assets/app_index.js @@ -0,0 +1,29 @@ +// BetterDiscord's Injection Script (app.asar method) +const path = require("path"); +const electron = require("electron"); + + +// Never let a missing or broken BetterDiscord asar keep Discord from launching: +// this file is the app entry point, so an unhandled throw here bricks the client. +// The whole BetterDiscord load โ€” path resolution included โ€” is wrapped so any +// failure (e.g. an unset HOME) falls through to Discord's real app below. +try { + // The global BetterDiscord folder lives one directory above userData (the + // appData root). Electron gives the postfixed userData, so go up a directory. + let userConfig = path.join(electron.app.getPath("userData"), ".."); + + // If we're on Linux there are a couple cases to deal with + if (process.platform !== "win32" && process.platform !== "darwin") { + // Use || instead of ?? because a falsey value of "" is invalid per XDG spec. + // os.homedir() resolves the home directory even if the HOME env var is unset. + const homeDir = process.env.HOME || require("os").homedir(); + userConfig = process.env.XDG_CONFIG_HOME || path.join(homeDir, ".config"); + } + require(path.join(userConfig, "BetterDiscord", "data", "betterdiscord.asar")); +} +catch (error) { + console.error("Failed to load BetterDiscord:", error); +} + +// Hand off to Discord's real (renamed) app entry point +module.exports = require("../betterdiscord.app.asar"); \ No newline at end of file diff --git a/internal/discord/assets/app_package.json b/internal/discord/assets/app_package.json new file mode 100644 index 0000000..c86ce6c --- /dev/null +++ b/internal/discord/assets/app_package.json @@ -0,0 +1 @@ +{"main": "./index.js"} \ No newline at end of file diff --git a/internal/discord/assets/injection.js b/internal/discord/assets/injection.js deleted file mode 100644 index 55f2f3b..0000000 --- a/internal/discord/assets/injection.js +++ /dev/null @@ -1,18 +0,0 @@ -// BetterDiscord's Injection Script -const path = require("path"); -const electron = require("electron"); - -// Windows and macOS both use the fixed global BetterDiscord folder but -// Electron gives the postfixed version of userData, so go up a directory -let userConfig = path.join(electron.app.getPath("userData"), ".."); - -// If we're on Linux there are a couple cases to deal with -if (process.platform !== "win32" && process.platform !== "darwin") { - // Use || instead of ?? because a falsey value of "" is invalid per XDG spec - userConfig = process.env.XDG_CONFIG_HOME || path.join(process.env.HOME, ".config"); -} - -require(path.join(userConfig, "BetterDiscord", "data", "betterdiscord.asar")); - -// Discord's Default Export -module.exports = require("./core.asar"); \ No newline at end of file diff --git a/internal/discord/injection.go b/internal/discord/injection.go index 905b7c8..bfabd3e 100644 --- a/internal/discord/injection.go +++ b/internal/discord/injection.go @@ -2,59 +2,238 @@ package discord import ( _ "embed" + "fmt" "os" "path/filepath" - "strings" "github.com/betterdiscord/cli/internal/betterdiscord" "github.com/betterdiscord/cli/internal/output" + "github.com/betterdiscord/cli/internal/utils" ) -//go:embed assets/injection.js -var injectionScript string +//go:embed assets/app_index.js +var appIndexScript string +//go:embed assets/app_package.json +var appPackageJSON string + +// errIfSnap rejects Snap installs with a clear, actionable message: their +// read-only squashfs mount can't host the app.asar shadow. It's called at the +// start of the install/uninstall/repair flows (before Discord is stopped, so an +// unsupported install never needlessly kills a running client) and again in +// inject/uninject as a backstop for any direct callers. +func (discord *DiscordInstall) errIfSnap() error { + if !discord.IsSnap { + return nil + } + output.Printf("โŒ Snap installs are not supported\n") + output.Printf(" The read-only Snap mount cannot host the BetterDiscord injection.\n") + return fmt.Errorf("snap installs are not supported") +} + +// probeWritable verifies dir accepts writes before we perform any destructive +// operation, by creating and removing a unique throwaway file. This is the +// elevation trigger: a failure here means we abort before touching the bundle. +// A unique name (os.CreateTemp) avoids colliding with or clobbering an existing +// file and is safe under concurrent probes. +func probeWritable(dir string) error { + f, err := os.CreateTemp(dir, ".bd-write-probe-*") + if err != nil { + return err + } + // Writability is already proven by the successful create; cleanup is + // best-effort and must not turn a writable dir into a probe failure. + _ = f.Close() + _ = os.Remove(f.Name()) + return nil +} + +// inject shadows Discord's app.asar: it preserves the original as +// betterdiscord.app.asar and drops an `app/` entry directory that loads +// BetterDiscord and then the preserved app. The operation is transactional โ€” +// any failure after the rename rolls back to the original state. +// +// bd is accepted for call-site symmetry with the install flow but unused: the +// injection script resolves the BetterDiscord folder at runtime. func (discord *DiscordInstall) inject(bd *betterdiscord.BDInstall) error { + resources := discord.ResourcesPath + if resources == "" { + return fmt.Errorf("cannot inject: resources path is empty") + } + + // Backstop: the install flow rejects Snap before stopping Discord, but guard + // here too so any direct caller gets the same actionable error rather than the + // generic permission failure the writability probe would raise below. + if err := discord.errIfSnap(); err != nil { + return err + } + + originalAsar := filepath.Join(resources, "app.asar") + preservedAsar := filepath.Join(resources, "betterdiscord.app.asar") + appDir := filepath.Join(resources, "app") + + // Probe writability before the destructive rename so we never leave a + // half-modified bundle on a read-only/permission-denied target. + if err := probeWritable(resources); err != nil { + output.Printf("โŒ Cannot write to %s\n", resources) + output.Printf(" %s\n", err.Error()) + return err + } + + // Preserve the original app.asar (idempotent, guarded). + // + // A live app.asar is always Discord's current app and takes priority: it + // must be renamed away or it would shadow our app/ folder (Electron loads + // app.asar before app/), silently disabling BetterDiscord. If a preserved + // copy is also present โ€” e.g. Discord repaired/reinstalled over a previous + // injection โ€” that copy is stale, so we discard it and re-preserve the live + // app. Only when there is no live app.asar do we treat an existing preserved + // copy as the (already-injected) source of truth and leave it be. + switch { + case utils.Exists(originalAsar): + if utils.Exists(preservedAsar) { + if err := os.Remove(preservedAsar); err != nil { + output.Printf("โŒ Unable to replace stale %s\n", preservedAsar) + output.Printf(" %s\n", err.Error()) + return err + } + } + if err := os.Rename(originalAsar, preservedAsar); err != nil { + output.Printf("โŒ Unable to modify app.asar in %s\n", resources) + output.Printf(" Discord may still be running, please fully close it and try again.\n") + output.Printf(" %s\n", err.Error()) + return err + } + case utils.Exists(preservedAsar): + // Already preserved from a prior injection and no live app.asar; the + // archive is correct โ€” only the shadow app/ needs (re)writing below. + default: + return fmt.Errorf("no app.asar found in %s", resources) + } + + // Roll back anything done after this point so a partial failure never leaves + // Discord without a loadable app. The restore is keyed on filesystem state, + // not on whether *this* call renamed: rollback's own RemoveAll(appDir) clears + // app/ even when re-injecting an already-injected install, so we must still + // restore app.asar from the preserved copy to keep Discord launchable. + rollback := func() { + err := os.RemoveAll(appDir) + if err != nil { + output.Printf("โŒ Rollback failed: unable to remove %s\n", appDir) + output.Printf(" %s\n", err.Error()) + } + if !utils.Exists(originalAsar) && utils.Exists(preservedAsar) { + if err := os.Rename(preservedAsar, originalAsar); err != nil { + output.Printf("โŒ Rollback failed: unable to restore app.asar in %s\n", resources) + output.Printf(" %s\n", err.Error()) + } + } + } - if err := os.WriteFile(filepath.Join(discord.CorePath, "index.js"), []byte(injectionScript), 0755); err != nil { - output.Printf("โŒ Unable to write index.js in %s\n", discord.CorePath) + if err := os.MkdirAll(appDir, 0755); err != nil { + output.Printf("โŒ Unable to create %s\n", appDir) output.Printf(" %s\n", err.Error()) + rollback() return err } - output.Printf("โœ… Injected into %s\n", discord.CorePath) + if err := os.WriteFile(filepath.Join(appDir, "package.json"), []byte(appPackageJSON), 0o644); err != nil { + output.Printf("โŒ Unable to write package.json in %s\n", appDir) + output.Printf(" %s\n", err.Error()) + rollback() + return err + } + + if err := os.WriteFile(filepath.Join(appDir, "index.js"), []byte(appIndexScript), 0o644); err != nil { + output.Printf("โŒ Unable to write index.js in %s\n", appDir) + output.Printf(" %s\n", err.Error()) + rollback() + return err + } + + if !utils.Exists(filepath.Join(appDir, "index.js")) || + !utils.Exists(filepath.Join(appDir, "package.json")) || + !utils.Exists(preservedAsar) { + rollback() + return fmt.Errorf("injection verification failed in %s", resources) + } + + output.Printf("โœ… Injected into %s\n", resources) return nil } +// uninject reverses inject: it removes the shadow `app/` directory and restores +// Discord's original app.asar from the preserved copy. func (discord *DiscordInstall) uninject() error { - indexFile := filepath.Join(discord.CorePath, "index.js") + resources := discord.ResourcesPath + if resources == "" { + return fmt.Errorf("cannot uninject: resources path is empty") + } - contents, err := os.ReadFile(indexFile) + // Backstop for direct callers; the uninstall/repair flows reject Snap before + // stopping Discord. Snap installs are never injectable, so there's nothing to + // revert โ€” report it explicitly rather than attempting filesystem mutations. + if err := discord.errIfSnap(); err != nil { + return err + } + + originalAsar := filepath.Join(resources, "app.asar") + preservedAsar := filepath.Join(resources, "betterdiscord.app.asar") + appDir := filepath.Join(resources, "app") - // First try to check the file, but if there's an issue we try to blindly overwrite below - if err == nil { - if !strings.Contains(strings.ToLower(string(contents)), "betterdiscord") { - output.Printf("โœ… No injection found for %s\n", discord.Channel.Name()) - return nil + // A clean install (only app.asar; no shadow app/ and no preserved copy) was + // never injected โ€” report a no-op instead of claiming a removal that didn't + // happen, which would mislead anyone troubleshooting uninstall/repair. + if !utils.Exists(appDir) && !utils.Exists(preservedAsar) { + output.Printf("โ„น๏ธ No injection found in %s\n", discord.Channel.Name()) + return nil + } + + // Restore Discord's original app.asar *before* removing the shadow app/. If the + // restore fails (e.g. a running Discord still locks the file), the injection is + // left fully intact and loadable rather than bricked with neither app.asar nor + // app/ present. + switch { + case utils.Exists(preservedAsar) && !utils.Exists(originalAsar): + // Normal revert: restore Discord's original app from the preserved copy. + if err := os.Rename(preservedAsar, originalAsar); err != nil { + output.Printf("โŒ Unable to restore app.asar in %s\n", resources) + output.Printf(" Discord may still be running, please fully close it and try again.\n") + output.Printf(" %s\n", err.Error()) + return err + } + case utils.Exists(preservedAsar): + // A live app.asar is already present (e.g. Discord repaired/reinstalled + // over the injection), so the preserved copy is stale. Remove it to fully + // revert and reclaim the space (100MB+). A failure here only leaves a + // harmless leftover โ€” Discord still launches โ€” so don't fail the uninstall. + if err := os.Remove(preservedAsar); err != nil { + output.Printf("โš ๏ธ Unable to remove stale %s\n", preservedAsar) + output.Printf(" %s\n", err.Error()) } } - if err := os.WriteFile(indexFile, []byte(`module.exports = require("./core.asar");`), 0o644); err != nil { - output.Printf("โŒ Unable to write file %s\n", indexFile) - output.Printf(" %s\n", err.Error()) - return err + // Original app restored (or the preserved copy was stale); now clear the shadow + // app/. A failure here is non-bricking โ€” Electron prefers the restored app.asar + // over app/ โ€” but still surface it so the leftover can be cleaned up. + if utils.Exists(appDir) { + if err := os.RemoveAll(appDir); err != nil { + output.Printf("โŒ Unable to remove %s\n", appDir) + output.Printf(" %s\n", err.Error()) + return err + } } - output.Printf("โœ… Removed from %s\n", discord.Channel.Name()) + output.Printf("โœ… Removed from %s\n", discord.Channel.Name()) return nil } -// TODO: consider putting this in the betterdiscord package +// IsInjected reports whether this install currently has the app.asar shadow in +// place: both our `app/index.js` entry and the preserved original must exist. func (discord *DiscordInstall) IsInjected() bool { - indexFile := filepath.Join(discord.CorePath, "index.js") - contents, err := os.ReadFile(indexFile) - if err != nil { + resources := discord.ResourcesPath + if resources == "" { return false } - lower := strings.ToLower(string(contents)) - return strings.Contains(lower, "betterdiscord") + return utils.Exists(filepath.Join(resources, "app", "index.js")) && utils.Exists(filepath.Join(resources, "betterdiscord.app.asar")) } diff --git a/internal/discord/injection_test.go b/internal/discord/injection_test.go new file mode 100644 index 0000000..967d9e8 --- /dev/null +++ b/internal/discord/injection_test.go @@ -0,0 +1,392 @@ +package discord + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/betterdiscord/cli/internal/models" + "github.com/betterdiscord/cli/internal/utils" +) + +// newResourcesDir creates a resources dir seeded with an app.asar of known +// content and returns the dir plus the original content. +func newResourcesDir(t *testing.T) (string, []byte) { + t.Helper() + resources := t.TempDir() + content := []byte("original discord app.asar") + if err := os.WriteFile(filepath.Join(resources, "app.asar"), content, 0o644); err != nil { + t.Fatalf("failed to seed app.asar: %v", err) + } + return resources, content +} + +func TestIsInjected(t *testing.T) { + resources := t.TempDir() + install := &DiscordInstall{ResourcesPath: resources, Channel: models.Stable} + + if install.IsInjected() { + t.Fatal("expected IsInjected false for a bare resources dir") + } + + // Only the app/ entry, no preserved asar โ†’ not injected. + if err := os.MkdirAll(filepath.Join(resources, "app"), 0o755); err != nil { + t.Fatalf("mkdir app: %v", err) + } + if err := os.WriteFile(filepath.Join(resources, "app", "index.js"), []byte("x"), 0o644); err != nil { + t.Fatalf("write index.js: %v", err) + } + if install.IsInjected() { + t.Fatal("expected IsInjected false without a preserved app.asar") + } + + // Add the preserved asar โ†’ injected. + if err := os.WriteFile(filepath.Join(resources, "betterdiscord.app.asar"), []byte("x"), 0o644); err != nil { + t.Fatalf("write preserved asar: %v", err) + } + if !install.IsInjected() { + t.Fatal("expected IsInjected true with app/index.js + preserved asar") + } +} + +func TestInject_Clean(t *testing.T) { + resources, original := newResourcesDir(t) + install := &DiscordInstall{ResourcesPath: resources, Channel: models.Stable} + + if err := install.inject(nil); err != nil { + t.Fatalf("inject() failed: %v", err) + } + + if utils.Exists(filepath.Join(resources, "app.asar")) { + t.Error("expected original app.asar to be renamed away") + } + preserved, err := os.ReadFile(filepath.Join(resources, "betterdiscord.app.asar")) + if err != nil { + t.Fatalf("preserved asar missing: %v", err) + } + if string(preserved) != string(original) { + t.Errorf("preserved asar content = %q, expected %q", preserved, original) + } + if !utils.Exists(filepath.Join(resources, "app", "index.js")) { + t.Error("app/index.js not written") + } + if !utils.Exists(filepath.Join(resources, "app", "package.json")) { + t.Error("app/package.json not written") + } + if !install.IsInjected() { + t.Error("expected IsInjected true after inject()") + } + + // index.js must reference the preserved app and the BD asar. + index, _ := os.ReadFile(filepath.Join(resources, "app", "index.js")) + if want := "../betterdiscord.app.asar"; !strings.Contains(string(index), want) { + t.Errorf("index.js missing %q", want) + } +} + +func TestInject_Idempotent(t *testing.T) { + resources, original := newResourcesDir(t) + install := &DiscordInstall{ResourcesPath: resources, Channel: models.Stable} + + if err := install.inject(nil); err != nil { + t.Fatalf("first inject() failed: %v", err) + } + // Corrupt the shadow index.js so we can confirm the second inject rewrites it + // without re-renaming (which would clobber the real, already-preserved asar). + if err := os.WriteFile(filepath.Join(resources, "app", "index.js"), []byte("stale"), 0o644); err != nil { + t.Fatalf("corrupt index.js: %v", err) + } + + if err := install.inject(nil); err != nil { + t.Fatalf("second inject() failed: %v", err) + } + + preserved, _ := os.ReadFile(filepath.Join(resources, "betterdiscord.app.asar")) + if string(preserved) != string(original) { + t.Errorf("preserved asar was clobbered on re-inject: got %q", preserved) + } + index, _ := os.ReadFile(filepath.Join(resources, "app", "index.js")) + if string(index) == "stale" { + t.Error("expected index.js to be rewritten on re-inject") + } + if utils.Exists(filepath.Join(resources, "app.asar")) { + t.Error("re-inject must not recreate a live app.asar") + } +} + +// Anomalous pre-state: a live app.asar AND a leftover betterdiscord.app.asar + +// app/ (e.g. Discord repaired/reinstalled over a prior injection). inject() must +// treat the live app.asar as authoritative โ€” discard the stale preserved copy, +// preserve the live app, and rename app.asar away so our app/ shadow loads +// (Electron would otherwise load the lingering app.asar and disable BD). +func TestInject_LiveAsarWinsOverStalePreserved(t *testing.T) { + resources := t.TempDir() + live := []byte("LIVE current app.asar") + stale := []byte("stale old preserved app") + if err := os.WriteFile(filepath.Join(resources, "app.asar"), live, 0o644); err != nil { + t.Fatalf("seed live app.asar: %v", err) + } + if err := os.WriteFile(filepath.Join(resources, "betterdiscord.app.asar"), stale, 0o644); err != nil { + t.Fatalf("seed stale preserved: %v", err) + } + if err := os.MkdirAll(filepath.Join(resources, "app"), 0o755); err != nil { + t.Fatalf("seed leftover app/: %v", err) + } + + install := &DiscordInstall{ResourcesPath: resources, Channel: models.Stable} + if err := install.inject(nil); err != nil { + t.Fatalf("inject: %v", err) + } + + // app.asar must be renamed away so it can't shadow app/. + if utils.Exists(filepath.Join(resources, "app.asar")) { + t.Error("live app.asar should have been renamed away") + } + // The preserved copy must be the LIVE app, not the stale leftover. + got, _ := os.ReadFile(filepath.Join(resources, "betterdiscord.app.asar")) + if string(got) != string(live) { + t.Errorf("preserved asar = %q, expected the live app %q", got, live) + } + if !install.IsInjected() { + t.Error("expected IsInjected after re-injecting over a repaired install") + } +} + +func TestUninject_RestoresExactly(t *testing.T) { + resources, original := newResourcesDir(t) + install := &DiscordInstall{ResourcesPath: resources, Channel: models.Stable} + + if err := install.inject(nil); err != nil { + t.Fatalf("inject() failed: %v", err) + } + if err := install.uninject(); err != nil { + t.Fatalf("uninject() failed: %v", err) + } + + restored, err := os.ReadFile(filepath.Join(resources, "app.asar")) + if err != nil { + t.Fatalf("app.asar not restored: %v", err) + } + if string(restored) != string(original) { + t.Errorf("restored app.asar = %q, expected %q", restored, original) + } + if utils.Exists(filepath.Join(resources, "betterdiscord.app.asar")) { + t.Error("preserved asar should be gone after uninject") + } + if utils.Exists(filepath.Join(resources, "app")) { + t.Error("shadow app/ should be removed after uninject") + } + if install.IsInjected() { + t.Error("expected IsInjected false after uninject()") + } +} + +// If Discord repaired/reinstalled over an injection, uninject encounters a live +// app.asar alongside a now-stale betterdiscord.app.asar. It must remove the +// stale copy (reclaiming 100MB+) and leave the live app untouched. +func TestUninject_RemovesStalePreservedWhenLiveAsarPresent(t *testing.T) { + resources := t.TempDir() + if err := os.WriteFile(filepath.Join(resources, "app.asar"), []byte("live"), 0o644); err != nil { + t.Fatalf("seed live app.asar: %v", err) + } + if err := os.WriteFile(filepath.Join(resources, "betterdiscord.app.asar"), []byte("stale"), 0o644); err != nil { + t.Fatalf("seed stale preserved: %v", err) + } + if err := os.MkdirAll(filepath.Join(resources, "app"), 0o755); err != nil { + t.Fatalf("seed app/: %v", err) + } + + install := &DiscordInstall{ResourcesPath: resources, Channel: models.Stable} + if err := install.uninject(); err != nil { + t.Fatalf("uninject: %v", err) + } + + if utils.Exists(filepath.Join(resources, "betterdiscord.app.asar")) { + t.Error("stale preserved copy should be removed when a live app.asar exists") + } + got, _ := os.ReadFile(filepath.Join(resources, "app.asar")) + if string(got) != "live" { + t.Errorf("app.asar = %q, expected the untouched live app", got) + } + if utils.Exists(filepath.Join(resources, "app")) { + t.Error("shadow app/ should be removed") + } + if install.IsInjected() { + t.Error("should not report injected after uninject") + } +} + +func TestUninject_NotInjectedIsNoop(t *testing.T) { + resources, original := newResourcesDir(t) + install := &DiscordInstall{ResourcesPath: resources, Channel: models.Stable} + + if err := install.uninject(); err != nil { + t.Fatalf("uninject() on a clean install failed: %v", err) + } + + // A never-injected install keeps its app.asar untouched. + got, _ := os.ReadFile(filepath.Join(resources, "app.asar")) + if string(got) != string(original) { + t.Errorf("uninject touched a clean app.asar: got %q", got) + } +} + +func TestInject_NoAppAsarErrors(t *testing.T) { + resources := t.TempDir() // empty, no app.asar + install := &DiscordInstall{ResourcesPath: resources, Channel: models.Stable} + + if err := install.inject(nil); err == nil { + t.Fatal("expected an error when no app.asar is present") + } + if utils.Exists(filepath.Join(resources, "app")) { + t.Error("no shadow app/ should be created when there's nothing to inject") + } +} + +func TestInject_EmptyResourcesPathErrors(t *testing.T) { + install := &DiscordInstall{ResourcesPath: "", Channel: models.Stable} + if err := install.inject(nil); err == nil { + t.Fatal("expected an error for an empty resources path (must not touch the cwd)") + } +} + +func TestUninject_EmptyResourcesPathErrors(t *testing.T) { + install := &DiscordInstall{ResourcesPath: "", Channel: models.Stable} + if err := install.uninject(); err == nil { + t.Fatal("expected an error for an empty resources path (must not RemoveAll the cwd)") + } +} + +// Rolling back a failed *re-injection* of an already-injected install must still +// leave Discord launchable: the preserve step is a no-op (no live app.asar), but +// rollback removes app/, so it must restore app.asar from the preserved copy. +func TestInject_RollbackRestoresLaunchableOnReinject(t *testing.T) { + resources := t.TempDir() + preserved := []byte("preserved discord app") + if err := os.WriteFile(filepath.Join(resources, "betterdiscord.app.asar"), preserved, 0o644); err != nil { + t.Fatalf("seed preserved: %v", err) + } + // Already-injected: app/ exists. Make index.js a directory so the index.js + // write fails *after* the (no-op) preserve step, forcing rollback. + if err := os.MkdirAll(filepath.Join(resources, "app", "index.js"), 0o755); err != nil { + t.Fatalf("seed app/index.js dir: %v", err) + } + + install := &DiscordInstall{ResourcesPath: resources, Channel: models.Stable} + if err := install.inject(nil); err == nil { + t.Fatal("expected inject to fail when app/index.js can't be written") + } + + // Discord must remain launchable: app.asar restored from the preserved copy. + restored, err := os.ReadFile(filepath.Join(resources, "app.asar")) + if err != nil { + t.Fatalf("app.asar not restored after rollback: %v", err) + } + if string(restored) != string(preserved) { + t.Errorf("restored app.asar = %q, expected %q", restored, preserved) + } + if utils.Exists(filepath.Join(resources, "app")) { + t.Error("shadow app/ should be removed by rollback") + } +} + +func TestInject_ProbeFailAbortsBeforeRename(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("chmod-based write denial is unreliable on Windows") + } + if os.Geteuid() == 0 { + t.Skip("running as root bypasses directory write permissions") + } + resources, original := newResourcesDir(t) + install := &DiscordInstall{ResourcesPath: resources, Channel: models.Stable} + + if err := os.Chmod(resources, 0o555); err != nil { + t.Fatalf("chmod: %v", err) + } + t.Cleanup(func() { _ = os.Chmod(resources, 0o755) }) + + if err := install.inject(nil); err == nil { + t.Fatal("expected inject to fail the writability probe") + } + + // The bundle must be untouched: app.asar still present, nothing renamed. + _ = os.Chmod(resources, 0o755) + got, err := os.ReadFile(filepath.Join(resources, "app.asar")) + if err != nil { + t.Fatalf("app.asar was disturbed by a probe-failed inject: %v", err) + } + if string(got) != string(original) { + t.Errorf("app.asar content changed: got %q", got) + } + if utils.Exists(filepath.Join(resources, "betterdiscord.app.asar")) { + t.Error("no rename should have happened after a probe failure") + } +} + +// Regression for the "invisible after injection" bug: injecting renames app.asar +// to betterdiscord.app.asar, so a resolver anchored only on app.asar would fail +// to find the install afterward โ€” breaking repair and, critically, uninstall. +func TestInjectThenResolve_RemainsDiscoverable(t *testing.T) { + tmpDir := t.TempDir() + root := filepath.Join(tmpDir, "Discord") + resources := filepath.Join(root, "app-1.0.9002", "resources") + writeAppAsar(t, resources) // pristine install + + if validateWindowsStyleInstall(root) == nil { + t.Fatal("precondition: pristine install should resolve") + } + + install := &DiscordInstall{ResourcesPath: resources, Channel: models.Stable} + if err := install.inject(nil); err != nil { + t.Fatalf("inject: %v", err) + } + + // The fix: it must still resolve from the top-level Discord root after injection. + resolved := validateWindowsStyleInstall(root) + if resolved == nil { + t.Fatal("injected install no longer resolves โ€” uninstall would be impossible") + } + if resolved.ResourcesPath != resources { + t.Errorf("ResourcesPath = %s, expected %s", resolved.ResourcesPath, resources) + } + if !resolved.IsInjected() { + t.Error("expected the resolved install to report IsInjected") + } + + // And uninstall works from the resolved install. + if err := resolved.uninject(); err != nil { + t.Fatalf("uninject: %v", err) + } + if !utils.Exists(filepath.Join(resources, "app.asar")) { + t.Error("app.asar not restored after uninject") + } +} + +func TestInject_RollbackOnMidOpFailure(t *testing.T) { + resources, original := newResourcesDir(t) + install := &DiscordInstall{ResourcesPath: resources, Channel: models.Stable} + + // Block mkdir(resources/app) by pre-creating a regular file at that path. + // This forces a failure *after* the app.asar rename, exercising rollback. + if err := os.WriteFile(filepath.Join(resources, "app"), []byte("blocker"), 0o644); err != nil { + t.Fatalf("seed blocker file: %v", err) + } + + if err := install.inject(nil); err == nil { + t.Fatal("expected inject to fail when app/ can't be created") + } + + // Rollback must restore the original app.asar and drop the preserved copy. + restored, err := os.ReadFile(filepath.Join(resources, "app.asar")) + if err != nil { + t.Fatalf("app.asar not restored after rollback: %v", err) + } + if string(restored) != string(original) { + t.Errorf("restored app.asar = %q, expected %q", restored, original) + } + if utils.Exists(filepath.Join(resources, "betterdiscord.app.asar")) { + t.Error("preserved asar should be gone after rollback") + } +} diff --git a/internal/discord/install.go b/internal/discord/install.go index 9771804..67449cb 100644 --- a/internal/discord/install.go +++ b/internal/discord/install.go @@ -1,23 +1,35 @@ package discord import ( + "os" + "path/filepath" + "strings" + "github.com/betterdiscord/cli/internal/betterdiscord" "github.com/betterdiscord/cli/internal/models" "github.com/betterdiscord/cli/internal/output" - "github.com/betterdiscord/cli/internal/utils" ) type DiscordInstall struct { - CorePath string `json:"corePath"` - Channel models.DiscordChannel `json:"channel"` - Version string `json:"version"` - IsFlatpak bool `json:"isFlatpak"` - IsSnap bool `json:"isSnap"` + ResourcesPath string `json:"resourcesPath"` + Channel models.DiscordChannel `json:"channel"` + Version string `json:"version"` + IsFlatpak bool `json:"isFlatpak"` + IsSnap bool `json:"isSnap"` } // InstallBD installs BetterDiscord into this Discord installation -func (discord *DiscordInstall) InstallBD() error { - bd := discord.GetBetterDiscordInstall() +func (discord *DiscordInstall) InstallBD(options models.InstallOptions) error { + // Reject Snap before doing anything (notably before stop()) so an unsupported + // install never needlessly kills a running Discord only to fail at inject(). + if err := discord.errIfSnap(); err != nil { + return err + } + + bd, err := discord.GetBetterDiscordInstall() + if err != nil { + return err + } // Make BetterDiscord folders output.Println("๐Ÿ›  Preparing BetterDiscord...") @@ -29,13 +41,21 @@ func (discord *DiscordInstall) InstallBD() error { // Download and write betterdiscord.asar output.Println("๐Ÿ“ฅ Downloading BetterDiscord...") - if err := bd.Download(); err != nil { + if err := bd.Download(options.UseDevBuild); err != nil { return err } output.Println("โœ… BetterDiscord downloaded") output.Blank() - // Write injection script to discord_desktop_core/index.js + // Discord locks app.asar while running, so it must be stopped before we can + // modify it. Capture the executable so it can be relaunched afterward. + exe, wasRunning, err := discord.stop() + if err != nil { + return err + } + output.Blank() + + // Shadow app.asar with our loader output.Println("๐Ÿ”Œ Injecting into Discord...") if err := discord.inject(bd); err != nil { return err @@ -43,80 +63,126 @@ func (discord *DiscordInstall) InstallBD() error { output.Println("โœ… Injection successful") output.Blank() - // Terminate and restart Discord if possible - output.Printf("๐Ÿ”„ Restarting %s...\n", discord.Channel.Name()) - if err := discord.restart(); err != nil { - return err + // Only relaunch what we stopped: if Discord wasn't running we leave it closed. + if options.RestartDiscord && wasRunning { + output.Printf("๐Ÿ”„ Restarting %s...\n", discord.Channel.Name()) + if err := discord.start(exe); err != nil { + return err + } + output.Blank() } - output.Blank() return nil } // UninstallBD removes BetterDiscord from this Discord installation -func (discord *DiscordInstall) UninstallBD() error { - output.Println("๐Ÿงน Removing injection...") - if err := discord.uninject(); err != nil { +func (discord *DiscordInstall) UninstallBD(options models.UninstallOptions) error { + // Reject Snap before stop() so an unsupported install isn't needlessly killed. + if err := discord.errIfSnap(); err != nil { + return err + } + + // Discord locks app.asar while running; stop it before reverting the injection. + exe, wasRunning, err := discord.stop() + if err != nil { return err } output.Blank() - output.Printf("๐Ÿ”„ Restarting %s...\n", discord.Channel.Name()) - if err := discord.restart(); err != nil { + output.Println("๐Ÿงน Removing injection...") + if err := discord.uninject(); err != nil { return err } output.Blank() + // If full-uninstall is requested, remove the global BetterDiscord install + if options.FullUninstall { + install, err := discord.GetBetterDiscordInstall() + if err != nil { + return err + } + if err := install.RemoveAll(); err != nil { + return err + } + output.Blank() + } + + // Only relaunch what we stopped: if Discord wasn't running we leave it closed. + if options.RestartDiscord && wasRunning { + output.Printf("๐Ÿ”„ Restarting %s...\n", discord.Channel.Name()) + if err := discord.start(exe); err != nil { + return err + } + output.Blank() + } + return nil } -// RepairBD repairs BetterDiscord for this Discord installation -func (discord *DiscordInstall) RepairBD() error { - if err := discord.UninstallBD(); err != nil { +// RepairBD repairs BetterDiscord for this Discord installation. It reverts the +// injection and cleans the requested data files, leaving BetterDiscord +// uninstalled; the caller then offers to reinstall. +func (discord *DiscordInstall) RepairBD(options models.RepairOptions) error { + // Reject Snap before stop() so an unsupported install isn't needlessly killed. + if err := discord.errIfSnap(); err != nil { return err } - // Gets the global BetterDiscord install - bd := betterdiscord.GetInstallation() + // Discord locks app.asar while running; stop it before reverting the injection. + exe, wasRunning, err := discord.stop() + if err != nil { + return err + } + output.Blank() - // Snaps and flatpaks get their own local BD install - if discord.IsFlatpak || discord.IsSnap { - segment := "config" - if discord.IsSnap { - segment = ".config" - } + output.Println("๐Ÿงน Removing injection...") + if err := discord.uninject(); err != nil { + return err + } + output.Blank() - configPath, err := utils.FindSegment(discord.CorePath, segment) - if err != nil { - return err - } - bd = betterdiscord.GetInstallation(configPath) + bd, err := discord.GetBetterDiscordInstall() + if err != nil { + return err } if err := bd.Repair(discord.Channel); err != nil { return err } + output.Blank() + + // Repair leaves Discord uninjected. If it was running, relaunch it (vanilla) + // so the user isn't left with a closed client; if they then accept the + // reinstall prompt, that flow stops and re-injects it. + if wasRunning { + output.Printf("๐Ÿ”„ Restarting %s...\n", discord.Channel.Name()) + if err := discord.start(exe); err != nil { + return err + } + output.Blank() + } return nil } -func (discord *DiscordInstall) GetBetterDiscordInstall() *betterdiscord.BDInstall { +func (discord *DiscordInstall) GetBetterDiscordInstall() (*betterdiscord.BDInstall, error) { // Gets the global BetterDiscord install bd := betterdiscord.GetInstallation() - // Snaps and flatpaks get their own local BD install - if discord.IsSnap || discord.IsFlatpak { - segment := "config" - if discord.IsSnap { - segment = ".config" - } - - configPath, err := utils.FindSegment(discord.CorePath, segment) + // Flatpaks get their own local BD folder. The resources path is in the + // read-only deployment tree, so we can't derive the sandbox config from it; + // instead we compute the stable ~/.var/app/{id}/config location from the + // channel. Inside the sandbox this dir is the app's $XDG_CONFIG_HOME, which + // is exactly where the injected index.js looks for BetterDiscord at runtime. + if discord.IsFlatpak { + home, err := os.UserHomeDir() if err != nil { - return nil + return nil, err } + id := "com.discordapp." + strings.ReplaceAll(discord.Channel.Name(), " ", "") + configPath := filepath.Join(home, ".var", "app", id, "config") bd = betterdiscord.GetInstallation(configPath) } - return bd + return bd, nil } diff --git a/internal/discord/install_test.go b/internal/discord/install_test.go new file mode 100644 index 0000000..3db2a9f --- /dev/null +++ b/internal/discord/install_test.go @@ -0,0 +1,91 @@ +package discord + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/betterdiscord/cli/internal/models" +) + +// UninstallBD with neither full-uninstall nor restart reverts the app.asar +// shadow without removing the global BD folder or relaunching Discord. (Discord +// isn't running in the test, so the stop() step is a no-op.) +func TestUninstallBD_UninjectOnly(t *testing.T) { + resources := t.TempDir() + // Seed an injected state: preserved asar + shadow app/ entry. + original := []byte("original app.asar") + if err := os.WriteFile(filepath.Join(resources, "betterdiscord.app.asar"), original, 0o644); err != nil { + t.Fatalf("seed preserved asar: %v", err) + } + if err := os.MkdirAll(filepath.Join(resources, "app"), 0o755); err != nil { + t.Fatalf("mkdir app: %v", err) + } + if err := os.WriteFile(filepath.Join(resources, "app", "index.js"), []byte("x"), 0o644); err != nil { + t.Fatalf("seed index.js: %v", err) + } + + install := &DiscordInstall{ResourcesPath: resources, Channel: models.Stable} + if err := install.UninstallBD(models.UninstallOptions{FullUninstall: false, RestartDiscord: false}); err != nil { + t.Fatalf("UninstallBD() failed: %v", err) + } + + if install.IsInjected() { + t.Error("expected the shadow to be reverted after UninstallBD") + } + restored, err := os.ReadFile(filepath.Join(resources, "app.asar")) + if err != nil { + t.Fatalf("app.asar not restored: %v", err) + } + if string(restored) != string(original) { + t.Errorf("app.asar after uninstall = %q, expected %q", restored, original) + } +} + +func TestGetBetterDiscordInstall_Global(t *testing.T) { + install := &DiscordInstall{ResourcesPath: "/some/discord/core", Channel: models.Stable} + + bd, err := install.GetBetterDiscordInstall() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if bd == nil { + t.Fatal("expected a non-nil global BD install") + } +} + +// Flatpak's BD folder is recomputed as ~/.var/app/{id}/config/BetterDiscord from +// the channel, independent of the (read-only deployment) resources path. +func TestGetBetterDiscordInstall_FlatpakRecomputesDataRoot(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("uses POSIX-style flatpak paths") + } + home, err := os.UserHomeDir() + if err != nil { + t.Skipf("no home dir: %v", err) + } + + cases := []struct { + channel models.DiscordChannel + id string + }{ + {models.Stable, "com.discordapp.Discord"}, + {models.Canary, "com.discordapp.DiscordCanary"}, + {models.PTB, "com.discordapp.DiscordPTB"}, + } + for _, tc := range cases { + // A resources path in the read-only deployment tree (no "config" segment). + resources := "/var/lib/flatpak/app/" + tc.id + "/current/active/files/discord/resources" + install := &DiscordInstall{ResourcesPath: resources, Channel: tc.channel, IsFlatpak: true} + + bd, err := install.GetBetterDiscordInstall() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := filepath.Join(home, ".var", "app", tc.id, "config", "BetterDiscord") + if bd.Root() != want { + t.Errorf("channel %v: Root() = %s, expected %s", tc.channel, bd.Root(), want) + } + } +} diff --git a/internal/discord/paths.go b/internal/discord/paths.go index 870af2b..623e536 100644 --- a/internal/discord/paths.go +++ b/internal/discord/paths.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/betterdiscord/cli/internal/models" + "github.com/betterdiscord/cli/internal/utils" ) var searchPaths []string @@ -28,7 +29,7 @@ func GetAllInstalls() map[models.DiscordChannel][]*DiscordInstall { } func GetVersion(proposed string) string { - for folder := range strings.SplitSeq(proposed, string(filepath.Separator)) { + for folder := range strings.SplitSeq(filepath.ToSlash(proposed), "/") { if version := versionRegex.FindString(folder); version != "" { return version } @@ -37,9 +38,25 @@ func GetVersion(proposed string) string { } func GetChannel(proposed string) models.DiscordChannel { - for folder := range strings.SplitSeq(proposed, string(filepath.Separator)) { + // Iterate from the leaf toward the root: the channel identifier always sits + // closest to the leaf (e.g. `.../discordcanary/app-x/resources`), so scanning + // backwards avoids false matches on a parent segment that happens to contain a + // channel name (e.g. a home dir at `/home/discord`). + // Normalize to forward slashes before splitting so a Windows path that mixes + // separators (backslashes and forward slashes, which the OS treats + // interchangeably) still segments cleanly. + segments := strings.Split(filepath.ToSlash(proposed), "/") + + for _, segment := range slices.Backward(segments) { + // Normalize the segment so macOS bundle names ("Discord Canary.app") and + // flatpak channel dirs ("discord-canary") both match the channel names + // ("discordcanary"). + normalized := strings.ToLower(segment) + normalized = strings.TrimSuffix(normalized, ".app") + normalized = strings.ReplaceAll(normalized, " ", "") + normalized = strings.ReplaceAll(normalized, "-", "") for _, channel := range models.Channels { - if strings.ToLower(folder) == strings.ReplaceAll(strings.ToLower(channel.Name()), " ", "") { + if normalized == strings.ReplaceAll(strings.ToLower(channel.Name()), " ", "") { return channel } } @@ -49,7 +66,7 @@ func GetChannel(proposed string) models.DiscordChannel { func GetSuggestedPath(channel models.DiscordChannel) string { if len(allDiscordInstalls[channel]) > 0 { - return allDiscordInstalls[channel][0].CorePath + return allDiscordInstalls[channel][0].ResourcesPath } return "" } @@ -61,7 +78,7 @@ func AddCustomPath(proposed string) *DiscordInstall { } // Check if this already exists in our list and return reference - index := slices.IndexFunc(allDiscordInstalls[result.Channel], func(d *DiscordInstall) bool { return d.CorePath == result.CorePath }) + index := slices.IndexFunc(allDiscordInstalls[result.Channel], func(d *DiscordInstall) bool { return d.ResourcesPath == result.ResourcesPath }) if index >= 0 { return allDiscordInstalls[result.Channel][index] } @@ -75,7 +92,7 @@ func AddCustomPath(proposed string) *DiscordInstall { func ResolvePath(proposed string) *DiscordInstall { for channel := range allDiscordInstalls { - index := slices.IndexFunc(allDiscordInstalls[channel], func(d *DiscordInstall) bool { return d.CorePath == proposed }) + index := slices.IndexFunc(allDiscordInstalls[channel], func(d *DiscordInstall) bool { return d.ResourcesPath == proposed }) if index >= 0 { return allDiscordInstalls[channel][index] } @@ -88,13 +105,9 @@ func ResolvePath(proposed string) *DiscordInstall { func sortInstalls() { for channel := range allDiscordInstalls { slices.SortFunc(allDiscordInstalls[channel], func(a, b *DiscordInstall) int { - switch { - case a.Version > b.Version: - return -1 - case b.Version > a.Version: - return 1 - } - return 0 + // Descending (highest version first) with a numeric compare so + // e.g. 1.0.10000 sorts above 1.0.9999. + return utils.CompareVersions(b.Version, a.Version) }) } } diff --git a/internal/discord/paths_common.go b/internal/discord/paths_common.go index 32a60e0..63bf890 100644 --- a/internal/discord/paths_common.go +++ b/internal/discord/paths_common.go @@ -3,209 +3,190 @@ package discord import ( - "io/fs" + "encoding/json" "os" "path/filepath" - "sort" "strings" + "github.com/betterdiscord/cli/internal/models" "github.com/betterdiscord/cli/internal/utils" ) -// validateWindowsStyleInstall validates a Windows-style Discord installation path. -// This is used for native Windows installs and WSL installs that point to Windows Discord. -// Windows Discord has a nested structure: Discord/app-1.0.9002/modules/discord_desktop_core-1/discord_desktop_core -func validateWindowsStyleInstall(proposed string) *DiscordInstall { - var finalPath = "" - var selected = filepath.Base(proposed) - - if strings.HasPrefix(selected, "Discord") { - // Get version dir like app-1.0.9002 - dFiles, err := os.ReadDir(proposed) - if err != nil { - return nil - } - - candidates := utils.Filter(dFiles, func(file fs.DirEntry) bool { - return file.IsDir() && versionRegex.MatchString(file.Name()) - }) - if len(candidates) == 0 { - return nil - } - sort.Slice(candidates, func(i, j int) bool { return candidates[i].Name() < candidates[j].Name() }) - versionDir := candidates[len(candidates)-1].Name() +// buildInfo mirrors the fields we care about in Discord's resources/build_info.json. +type buildInfo struct { + ReleaseChannel string `json:"releaseChannel"` + Version string `json:"version"` +} - // Get core wrap like discord_desktop_core-1 - dFiles, err = os.ReadDir(filepath.Join(proposed, versionDir, "modules")) - if err != nil { - return nil - } - candidates = utils.Filter(dFiles, func(file fs.DirEntry) bool { - return file.IsDir() && strings.HasPrefix(file.Name(), "discord_desktop_core") - }) - if len(candidates) == 0 { - return nil - } - coreWrap := candidates[len(candidates)-1].Name() +// readBuildInfo reads resources/build_info.json. The second return is false when +// the file is absent or unparseable, so callers can fall back to path parsing. +func readBuildInfo(resourcesDir string) (buildInfo, bool) { + data, err := os.ReadFile(filepath.Join(resourcesDir, "build_info.json")) + if err != nil { + return buildInfo{}, false + } - finalPath = filepath.Join(proposed, versionDir, "modules", coreWrap, "discord_desktop_core") + var info buildInfo + if err := json.Unmarshal(data, &info); err != nil { + return buildInfo{}, false } - // Handle app-* directories (e.g., app-1.0.9002) - if strings.HasPrefix(selected, "app-") { - dFiles, err := os.ReadDir(filepath.Join(proposed, "modules")) - if err != nil { - return nil - } + return info, true +} - candidates := utils.Filter(dFiles, func(file fs.DirEntry) bool { - return file.IsDir() && strings.HasPrefix(file.Name(), "discord_desktop_core") - }) - if len(candidates) == 0 { - return nil - } - coreWrap := candidates[len(candidates)-1].Name() - finalPath = filepath.Join(proposed, "modules", coreWrap, "discord_desktop_core") - } +// hasDiscordApp reports whether dir is a Discord `resources` directory โ€” that +// is, it contains Discord's app archive in *either* state: +// - `app.asar` โ€” a pristine (or freshly updated) install, and +// - `betterdiscord.app.asar` โ€” the original preserved after BetterDiscord +// injects its shadow `app/` folder (at which point `app.asar` no longer +// exists). +// +// Checking both is essential: once injected, an install would otherwise stop +// resolving, so users could no longer repair or โ€” critically โ€” uninstall it. +func hasDiscordApp(dir string) bool { + return utils.Exists(filepath.Join(dir, "app.asar")) || utils.Exists(filepath.Join(dir, "betterdiscord.app.asar")) +} - if selected == "discord_desktop_core" { - finalPath = proposed +// latestAppDir returns the highest-versioned `app-{version}` child of base whose +// resources dir actually holds a Discord app, or "" when none qualify. Skipping +// broken/incomplete version dirs (e.g. from an interrupted Discord update) lets +// resolution fall back to a slightly older but valid install instead of failing. +// Sorting is numeric so 1.0.10000 beats 1.0.9999. +func latestAppDir(base string) string { + entries, err := os.ReadDir(base) + if err != nil { + return "" } - // Verify the path and core.asar exist - if utils.Exists(finalPath) && utils.Exists(filepath.Join(finalPath, "core.asar")) { - return &DiscordInstall{ - CorePath: finalPath, - Channel: GetChannel(finalPath), - Version: GetVersion(finalPath), - IsFlatpak: false, - IsSnap: false, + bestName := "" + bestVersion := "" + for _, entry := range entries { + if !entry.IsDir() || !strings.HasPrefix(entry.Name(), "app-") { + continue + } + version := strings.TrimPrefix(entry.Name(), "app-") + if !versionRegex.MatchString(version) { + continue + } + if !hasDiscordApp(filepath.Join(base, entry.Name(), "resources")) { + continue + } + if bestName == "" || utils.CompareVersions(version, bestVersion) > 0 { + bestName, bestVersion = entry.Name(), version } } - return nil + return bestName } -// validateUnixStyleInstall validates a Unix-style Discord installation path (Linux native, macOS). -// Unix Discord sometimes has a flatter structure: discord/0.0.35/modules/discord_desktop_core -// But sometimes it has the same pattern as Windows. This function detects both patterns and also -// identifies Flatpak and Snap installations if requested. -func validateUnixStyleInstall(proposed string, detectFlatpak bool, detectSnap bool) *DiscordInstall { - var finalPath = "" - var selected = filepath.Base(proposed) - - // Flatpak specific handling - if strings.HasPrefix(selected, "com.discordapp") { - channelPaths, err := os.ReadDir(filepath.Join(proposed, "config")) - if err != nil { - return nil - } - - candidates := utils.Filter(channelPaths, func(file fs.DirEntry) bool { - return file.IsDir() && strings.HasPrefix(file.Name(), "discord") - }) +// isSnapPath reports whether a resolved resources path lives under a Snap mount +// (/snap/โ€ฆ or /var/lib/snapd/snap/โ€ฆ). Anchoring to the mount points avoids +// false-positives on unrelated paths that merely contain a "snap" segment โ€” e.g. +// the home directory of a user named "snap" (/home/snap/โ€ฆ). +func isSnapPath(path string) bool { + sep := string(filepath.Separator) + return strings.HasPrefix(path, "snap"+sep) || + strings.HasPrefix(path, sep+"snap"+sep) || + strings.HasPrefix(path, sep+"var"+sep+"lib"+sep+"snapd"+sep+"snap"+sep) +} - if len(candidates) == 0 { - return nil - } +// resolveResources locates the Discord `resources` directory (holding Discord's +// app archive โ€” see hasDiscordApp) from a variety of proposed inputs, returning +// "" when none is found: +// - a resources dir itself (or macOS Contents/Resources) โ€” the archive is directly inside +// - an `app-{version}` dir โ€” drills into its `resources` +// - a dir that directly contains a `resources` child (flatpak files/{channel-}) +// - a base holding `app-{version}` dirs (Discord root / channel config dir) โ€” picks latest +func resolveResources(proposed string) string { + if proposed == "" { + return "" + } - // Assume the first candidate is the correct one (e.g., discord or discordcanary) - // Then set proposed and select so the remaining logic can find the core.asar - // - // TODO: This entire validation function could be refactored to use this fall-through logic - // instead of trying to fully handle each pattern, but for now this is a simple way to support - // Flatpak's extra nesting without breaking existing validations - channelPath := candidates[0].Name() - proposed = filepath.Join(proposed, "config", channelPath) - selected = channelPath - } - - if strings.HasPrefix(strings.ToLower(selected), "discord") { - // Get version dir like 0.0.35 - dFiles, err := os.ReadDir(proposed) - if err != nil { - return nil - } + // The proposed path is already the resources dir (or macOS Contents/Resources). + if hasDiscordApp(proposed) { + return proposed + } - candidates := utils.Filter(dFiles, func(file fs.DirEntry) bool { - return file.IsDir() && versionRegex.MatchString(file.Name()) - }) - if len(candidates) == 0 { - return nil + if strings.HasPrefix(filepath.Base(proposed), "app-") { + if res := filepath.Join(proposed, "resources"); hasDiscordApp(res) { + return res } - sort.Slice(candidates, func(i, j int) bool { return candidates[i].Name() < candidates[j].Name() }) - versionDir := candidates[len(candidates)-1].Name() + return "" + } - // Get core wrap like discord_desktop_core-1 - dFiles, err = os.ReadDir(filepath.Join(proposed, versionDir, "modules")) - if err != nil { - return nil - } - candidates = utils.Filter(dFiles, func(file fs.DirEntry) bool { - return file.IsDir() && strings.HasPrefix(file.Name(), "discord_desktop_core") - }) + // A dir with a direct `resources` child (flatpak files/{channel-}). + if res := filepath.Join(proposed, "resources"); hasDiscordApp(res) { + return res + } - if len(candidates) == 0 { - return nil - } + // A macOS app bundle: app.asar lives in Contents/Resources. + if res := filepath.Join(proposed, "Contents", "Resources"); hasDiscordApp(res) { + return res + } - // If no core wrap is found, assume the structure is flatter and point directly to discord_desktop_core - coreWrap := candidates[len(candidates)-1].Name() - if coreWrap == "discord_desktop_core" { - finalPath = filepath.Join(proposed, versionDir, "modules", "discord_desktop_core") - } else { - finalPath = filepath.Join(proposed, versionDir, "modules", coreWrap, "discord_desktop_core") + // A base containing versioned app dirs (Windows Discord root, Linux channel dir). + if latest := latestAppDir(proposed); latest != "" { + if res := filepath.Join(proposed, latest, "resources"); hasDiscordApp(res) { + return res } } - // Handle version directories (e.g. app-0.0.35, 0.0.35) - if strings.HasPrefix(selected, "app-") || versionRegex.MatchString(selected) { - dFiles, err := os.ReadDir(filepath.Join(proposed, "modules")) - if err != nil { - return nil - } + return "" +} - candidates := utils.Filter(dFiles, func(file fs.DirEntry) bool { - return file.IsDir() && strings.HasPrefix(file.Name(), "discord_desktop_core") - }) +// newResourcesInstall builds a DiscordInstall for a resolved resources dir, +// preferring build_info.json for channel/version and falling back to the path. +func newResourcesInstall(resourcesDir string) *DiscordInstall { + channel := GetChannel(resourcesDir) + version := GetVersion(resourcesDir) - if len(candidates) == 0 { - return nil + if info, ok := readBuildInfo(resourcesDir); ok { + if info.ReleaseChannel != "" { + channel = models.ParseChannel(info.ReleaseChannel) } - - // If no core wrap is found, assume the structure is flatter and point directly to discord_desktop_core - coreWrap := candidates[len(candidates)-1].Name() - if coreWrap == "discord_desktop_core" { - finalPath = filepath.Join(proposed, "modules", "discord_desktop_core") - } else { - finalPath = filepath.Join(proposed, "modules", coreWrap, "discord_desktop_core") + if info.Version != "" { + version = info.Version } } - if selected == "discord_desktop_core" { - finalPath = proposed + return &DiscordInstall{ + ResourcesPath: resourcesDir, + Channel: channel, + Version: version, } +} - // Verify the path and core.asar exist - if utils.Exists(finalPath) && utils.Exists(filepath.Join(finalPath, "core.asar")) { - isFlatpak := false - isSnap := false +// validateWindowsStyleInstall validates a Windows-style install (native Windows +// and WSL pointing at Windows Discord). The new updater lays out installs as +// Discord/app-{version}/resources/app.asar. +func validateWindowsStyleInstall(proposed string) *DiscordInstall { + resources := resolveResources(proposed) + if resources == "" { + return nil + } + return newResourcesInstall(resources) +} - if detectFlatpak { - isFlatpak = strings.Contains(finalPath, "com.discordapp.") - } - if detectSnap { - isSnap = strings.Contains(finalPath, "snap/") - } +// validateUnixStyleInstall validates a Unix-style install (Linux native, macOS). +// Linux native mirrors the Windows layout under the config dir +// (~/.config/{channel}/app-{version}/resources); macOS keeps app.asar directly in +// the bundle's Contents/Resources. Flatpak/Snap are flagged via the resolved path. +func validateUnixStyleInstall(proposed string, detectFlatpak bool, detectSnap bool) *DiscordInstall { + resources := resolveResources(proposed) + if resources == "" { + return nil + } - return &DiscordInstall{ - CorePath: finalPath, - Channel: GetChannel(finalPath), - Version: GetVersion(finalPath), - IsFlatpak: isFlatpak, - IsSnap: isSnap, - } + install := newResourcesInstall(resources) + + // Heuristic: infer packaging format from the resolved path. These substring + // checks match the real Flatpak/Snap layouts in practice. + if detectFlatpak { + install.IsFlatpak = strings.Contains(resources, "com.discordapp.") + } + if detectSnap { + install.IsSnap = isSnapPath(resources) } - return nil + return install } diff --git a/internal/discord/paths_common_test.go b/internal/discord/paths_common_test.go new file mode 100644 index 0000000..9d3077b --- /dev/null +++ b/internal/discord/paths_common_test.go @@ -0,0 +1,323 @@ +package discord + +import ( + "os" + "path/filepath" + "testing" + + "github.com/betterdiscord/cli/internal/models" +) + +// writeAppAsar creates a resources dir seeded with an app.asar. +func writeAppAsar(t *testing.T, resourcesDir string) { + t.Helper() + if err := os.MkdirAll(resourcesDir, 0755); err != nil { + t.Fatalf("Failed to create resources dir: %v", err) + } + if err := os.WriteFile(filepath.Join(resourcesDir, "app.asar"), []byte("test"), 0644); err != nil { + t.Fatalf("Failed to write app.asar: %v", err) + } +} + +// writeInjectedResources creates a resources dir in the *injected* state: +// app.asar has been renamed to betterdiscord.app.asar and a shadow app/ exists. +func writeInjectedResources(t *testing.T, resourcesDir string) { + t.Helper() + if err := os.MkdirAll(filepath.Join(resourcesDir, "app"), 0755); err != nil { + t.Fatalf("Failed to create app dir: %v", err) + } + if err := os.WriteFile(filepath.Join(resourcesDir, "betterdiscord.app.asar"), []byte("preserved"), 0644); err != nil { + t.Fatalf("Failed to write preserved asar: %v", err) + } + if err := os.WriteFile(filepath.Join(resourcesDir, "app", "index.js"), []byte("// bd"), 0644); err != nil { + t.Fatalf("Failed to write index.js: %v", err) + } +} + +// Regression: an install stays resolvable after injection (app.asar renamed to +// betterdiscord.app.asar). If it didn't, users couldn't repair or uninstall it. +func TestValidateWindowsStyleInstall_ResolvesInjected(t *testing.T) { + tmpDir := t.TempDir() + root := filepath.Join(tmpDir, "Discord") + resources := filepath.Join(root, "app-1.0.9002", "resources") + writeInjectedResources(t, resources) // no app.asar, only betterdiscord.app.asar + + result := validateWindowsStyleInstall(root) + if result == nil { + t.Fatalf("injected install must still resolve for %s", root) + } + if result.ResourcesPath != resources { + t.Errorf("ResourcesPath = %s, expected %s", result.ResourcesPath, resources) + } +} + +func TestResolveResources_InjectedResourcesDir(t *testing.T) { + // Browsing/uninstalling straight to an injected resources dir must resolve. + resources := filepath.Join(t.TempDir(), "resources") + writeInjectedResources(t, resources) + + if got := resolveResources(resources); got != resources { + t.Errorf("resolveResources(injected) = %q, expected %q", got, resources) + } +} + +func TestValidateWindowsStyleInstall_FromDiscordRoot(t *testing.T) { + tmpDir := t.TempDir() + root := filepath.Join(tmpDir, "Discord") + resources := filepath.Join(root, "app-1.0.9002", "resources") + writeAppAsar(t, resources) + + result := validateWindowsStyleInstall(root) + if result == nil { + t.Fatalf("Expected install for %s", root) + } + if result.ResourcesPath != resources { + t.Errorf("ResourcesPath = %s, expected %s", result.ResourcesPath, resources) + } +} + +func TestValidateWindowsStyleInstall_PicksLatestVersion(t *testing.T) { + tmpDir := t.TempDir() + root := filepath.Join(tmpDir, "Discord") + // An older leftover version dir plus the current one. + writeAppAsar(t, filepath.Join(root, "app-1.0.9002", "resources")) + latest := filepath.Join(root, "app-1.0.10000", "resources") + writeAppAsar(t, latest) + + result := validateWindowsStyleInstall(root) + if result == nil { + t.Fatalf("Expected install for %s", root) + } + if result.ResourcesPath != latest { + t.Errorf("ResourcesPath = %s, expected latest %s", result.ResourcesPath, latest) + } +} + +func TestValidateWindowsStyleInstall_FromAppFolder(t *testing.T) { + tmpDir := t.TempDir() + versionDir := filepath.Join(tmpDir, "Discord", "app-1.0.9002") + resources := filepath.Join(versionDir, "resources") + writeAppAsar(t, resources) + + result := validateWindowsStyleInstall(versionDir) + if result == nil { + t.Fatalf("Expected install for %s", versionDir) + } + if result.ResourcesPath != resources { + t.Errorf("ResourcesPath = %s, expected %s", result.ResourcesPath, resources) + } +} + +func TestValidateWindowsStyleInstall_FromResourcesFolder(t *testing.T) { + resources := filepath.Join(t.TempDir(), "resources") + writeAppAsar(t, resources) + + result := validateWindowsStyleInstall(resources) + if result == nil { + t.Fatalf("Expected install for %s", resources) + } + if result.ResourcesPath != resources { + t.Errorf("ResourcesPath = %s, expected %s", result.ResourcesPath, resources) + } +} + +func TestValidateWindowsStyleInstall_MissingAsar(t *testing.T) { + tmpDir := t.TempDir() + root := filepath.Join(tmpDir, "Discord") + // resources dir exists but has no app.asar. + if err := os.MkdirAll(filepath.Join(root, "app-1.0.9002", "resources"), 0755); err != nil { + t.Fatalf("Failed to create resources dir: %v", err) + } + + if result := validateWindowsStyleInstall(root); result != nil { + t.Fatalf("Expected no install when app.asar is missing") + } +} + +func TestValidateUnixStyleInstall_FromChannelRoot(t *testing.T) { + tmpDir := t.TempDir() + root := filepath.Join(tmpDir, "discord") + resources := filepath.Join(root, "app-0.0.90", "resources") + writeAppAsar(t, resources) + + result := validateUnixStyleInstall(root, true, true) + if result == nil { + t.Fatalf("Expected install for %s", root) + } + if result.ResourcesPath != resources { + t.Errorf("ResourcesPath = %s, expected %s", result.ResourcesPath, resources) + } + if result.IsFlatpak || result.IsSnap { + t.Errorf("plain path should not flag flatpak/snap: %+v", result) + } +} + +func TestValidateUnixStyleInstall_FromVersionFolder(t *testing.T) { + tmpDir := t.TempDir() + versionDir := filepath.Join(tmpDir, "discord", "app-0.0.90") + resources := filepath.Join(versionDir, "resources") + writeAppAsar(t, resources) + + result := validateUnixStyleInstall(versionDir, true, true) + if result == nil { + t.Fatalf("Expected install for %s", versionDir) + } + if result.ResourcesPath != resources { + t.Errorf("ResourcesPath = %s, expected %s", result.ResourcesPath, resources) + } +} + +func TestValidateUnixStyleInstall_FlatpakDetection(t *testing.T) { + tmpDir := t.TempDir() + // Flatpak deployment layout: files/{channel-}/resources (no app-* segment). + resources := filepath.Join(tmpDir, "com.discordapp.Discord", "files", "discord", "resources") + writeAppAsar(t, resources) + + result := validateUnixStyleInstall(resources, true, false) + if result == nil { + t.Fatalf("Expected install for %s", resources) + } + if !result.IsFlatpak { + t.Fatalf("Expected flatpak detection for %s", resources) + } + if result.IsSnap { + t.Fatalf("Did not expect snap detection") + } +} + +func TestValidateUnixStyleInstall_MacOSBundle(t *testing.T) { + // macOS: app.asar lives in {Bundle}.app/Contents/Resources; channel/version + // come from build_info.json (the bundle name has a space and no version). + tmpDir := t.TempDir() + bundle := filepath.Join(tmpDir, "Discord Canary.app") + resources := filepath.Join(bundle, "Contents", "Resources") + writeAppAsar(t, resources) + if err := os.WriteFile(filepath.Join(resources, "build_info.json"), + []byte(`{"releaseChannel":"canary","version":"1.0.1"}`), 0644); err != nil { + t.Fatalf("write build_info: %v", err) + } + + // Resolving from the bundle path (as a user browsing to Discord.app would). + result := validateUnixStyleInstall(bundle, false, false) + if result == nil { + t.Fatalf("Expected install for bundle %s", bundle) + } + if result.ResourcesPath != resources { + t.Errorf("ResourcesPath = %s, expected %s", result.ResourcesPath, resources) + } + if result.Channel != models.Canary { + t.Errorf("Channel = %v, expected Canary", result.Channel) + } + if result.Version != "1.0.1" { + t.Errorf("Version = %q, expected 1.0.1", result.Version) + } +} + +func TestIsSnapPath(t *testing.T) { + sep := string(filepath.Separator) + tests := []struct { + name string + path string + want bool + }{ + {"snap mount", sep + filepath.Join("snap", "discord", "current", "resources"), true}, + {"snapd mount", sep + filepath.Join("var", "lib", "snapd", "snap", "discord", "resources"), true}, + {"user named snap", sep + filepath.Join("home", "snap", ".config", "discord", "resources"), false}, + {"mysnap segment", sep + filepath.Join("home", "u", "mysnap", "discord", "resources"), false}, + {"native config", sep + filepath.Join("home", "u", ".config", "discord", "app-1.0.1", "resources"), false}, + } + for _, tt := range tests { + if got := isSnapPath(tt.path); got != tt.want { + t.Errorf("%s: isSnapPath(%q) = %v, want %v", tt.name, tt.path, got, tt.want) + } + } +} + +// An interrupted Discord update can leave a higher-versioned app-* dir with a +// broken/empty resources folder next to a valid older one. Resolution must fall +// back to the valid older version rather than failing outright. +func TestValidateWindowsStyleInstall_SkipsBrokenLatestVersion(t *testing.T) { + tmpDir := t.TempDir() + root := filepath.Join(tmpDir, "Discord") + + valid := filepath.Join(root, "app-1.0.9002", "resources") + writeAppAsar(t, valid) + + // Newer version dir exists but its resources has no app.asar (broken update). + if err := os.MkdirAll(filepath.Join(root, "app-1.0.10000", "resources"), 0755); err != nil { + t.Fatalf("create broken version dir: %v", err) + } + + result := validateWindowsStyleInstall(root) + if result == nil { + t.Fatal("expected resolution to fall back to the valid older version") + } + if result.ResourcesPath != valid { + t.Errorf("ResourcesPath = %s, expected valid older %s", result.ResourcesPath, valid) + } +} + +func TestReadBuildInfo(t *testing.T) { + t.Run("present", func(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "build_info.json"), + []byte(`{"releaseChannel":"canary","version":"1.0.1234"}`), 0644); err != nil { + t.Fatalf("write build_info: %v", err) + } + info, ok := readBuildInfo(dir) + if !ok { + t.Fatal("expected ok=true for a present build_info.json") + } + if info.ReleaseChannel != "canary" || info.Version != "1.0.1234" { + t.Errorf("parsed = %+v", info) + } + }) + + t.Run("absent", func(t *testing.T) { + if _, ok := readBuildInfo(t.TempDir()); ok { + t.Error("expected ok=false when build_info.json is absent") + } + }) + + t.Run("malformed", func(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "build_info.json"), []byte("{not json"), 0644); err != nil { + t.Fatalf("write: %v", err) + } + if _, ok := readBuildInfo(dir); ok { + t.Error("expected ok=false for malformed build_info.json") + } + }) +} + +func TestNewResourcesInstall_PrefersBuildInfo(t *testing.T) { + // Path segments say stable/no-version, but build_info.json says canary/1.0.5. + resources := filepath.Join(t.TempDir(), "discord", "app-0.0.1", "resources") + writeAppAsar(t, resources) + if err := os.WriteFile(filepath.Join(resources, "build_info.json"), + []byte(`{"releaseChannel":"canary","version":"1.0.5"}`), 0644); err != nil { + t.Fatalf("write build_info: %v", err) + } + + install := newResourcesInstall(resources) + if install.Channel != models.Canary { + t.Errorf("Channel = %v, expected Canary from build_info", install.Channel) + } + if install.Version != "1.0.5" { + t.Errorf("Version = %q, expected 1.0.5 from build_info", install.Version) + } +} + +func TestNewResourcesInstall_FallsBackToPath(t *testing.T) { + // No build_info.json โ†’ channel/version come from the path. + resources := filepath.Join(t.TempDir(), "discordcanary", "app-0.0.90", "resources") + writeAppAsar(t, resources) + + install := newResourcesInstall(resources) + if install.Channel != models.Canary { + t.Errorf("Channel = %v, expected Canary from path", install.Channel) + } + if install.Version != "0.0.90" { + t.Errorf("Version = %q, expected 0.0.90 from path", install.Version) + } +} diff --git a/internal/discord/paths_darwin.go b/internal/discord/paths_darwin.go index 56a5f01..8e8f616 100644 --- a/internal/discord/paths_darwin.go +++ b/internal/discord/paths_darwin.go @@ -3,24 +3,30 @@ package discord import ( "os" "path/filepath" - "strings" "github.com/betterdiscord/cli/internal/models" ) func init() { - config, _ := os.UserConfigDir() - paths := []string{ - filepath.Join(config, "{channel}"), + home, err := os.UserHomeDir() + + // On macOS the app.asar lives inside the application bundle + // (Discord.app/Contents/Resources), not under Application Support. Search the + // standard install locations for each channel's bundle. + bases := []string{ + filepath.Join("/", "Applications"), + } + + // Only add ~/Applications when the home dir resolved; otherwise the join would + // produce a relative "Applications" and search the current working directory. + if err == nil && home != "" { + bases = append(bases, filepath.Join(home, "Applications")) } for _, channel := range models.Channels { - for _, path := range paths { - folder := strings.ReplaceAll(strings.ToLower(channel.Name()), " ", "") - searchPaths = append( - searchPaths, - strings.ReplaceAll(path, "{channel}", folder), - ) + bundle := channel.Name() + ".app" + for _, base := range bases { + searchPaths = append(searchPaths, filepath.Join(base, bundle)) } } diff --git a/internal/discord/paths_linux.go b/internal/discord/paths_linux.go index fc84ef0..8855f08 100644 --- a/internal/discord/paths_linux.go +++ b/internal/discord/paths_linux.go @@ -9,37 +9,42 @@ import ( "github.com/betterdiscord/cli/internal/wsl" ) +// Snap is intentionally omitted: its read-only squashfs mount can't host +// the app.asar shadow, so the new injection method does not support it. func init() { - config, _ := os.UserConfigDir() - home, _ := os.UserHomeDir() + config, errConfig := os.UserConfigDir() + home, errHome := os.UserHomeDir() + + // Flatpak (global). The app.asar lives in the read-only deployment files. + // Example: `/var/lib/flatpak/app/com.discordapp.DiscordCanary/current/active/files/discord-canary/resources/app.asar`. + // This has no home/config dependency, so it's always searched. paths := []string{ - // Native. Data is stored under `~/.config`. - // Example: `~/.config/discordcanary`. - // Core: `~/.config/discordcanary/0.0.90/modules/discord_desktop_core/core.asar`. - // Updated Core: `~/.config/discordcanary/app-0.0.90/modules/discord_desktop_core-1/discord_desktop_core/core.asar`. - filepath.Join(config, "{channel}"), + filepath.Join("/var", "lib", "flatpak", "app", "com.discordapp.{CHANNEL}", "current", "active", "files", "{channel-}", "resources"), + } - // Flatpak. These user data paths are universal for all Flatpak installations on all machines. - // Example: `.var/app/com.discordapp.DiscordCanary/config/discordcanary`. - // Core: `.var/app/com.discordapp.DiscordCanary/config/discordcanary/0.0.90/modules/discord_desktop_core/core.asar` - // Updated Core: `.var/app/com.discordapp.DiscordCanary/config/discordcanary/app-0.0.90/modules/discord_desktop_core-1/discord_desktop_core/core.asar`. - filepath.Join(home, ".var", "app", "com.discordapp.{CHANNEL}", "config", "{channel}"), + // Only search config/home-relative locations when those dirs resolved; + // otherwise the joins would produce relative paths anchored at the current + // working directory. + + // Native. The new updater lays out versioned app dirs under `~/.config`. + // Example: `~/.config/discordcanary`. + // Resources: `~/.config/discordcanary/app-0.0.90/resources/app.asar`. + if errConfig == nil && config != "" { + paths = append(paths, filepath.Join(config, "{channel}")) + } - // Snap. Just like with Flatpaks, these paths are universal for all Snap installations. - // Example: `snap/discord/current/.config/discord`. - // Example: `snap/discord-canary/current/.config/discordcanary`. - // Core: `snap/discord-canary/current/.config/discordcanary/0.0.90/modules/discord_desktop_core/core.asar`. - // Updated Core: `snap/discord-canary/current/.config/discordcanary/app-0.0.90/modules/discord_desktop_core-1/discord_desktop_core/core.asar`. - // NOTE: Snap user data always exists, even when the Snap isn't mounted/running. - filepath.Join(home, "snap", "{channel-}", "current", ".config", "{channel}"), + // Flatpak (user). Same layout under the per-user flatpak tree (writable). + // Example: `~/.local/share/flatpak/app/com.discordapp.DiscordCanary/current/active/files/discord-canary/resources/app.asar`. + if errHome == nil && home != "" { + paths = append(paths, filepath.Join(home, ".local", "share", "flatpak", "app", "com.discordapp.{CHANNEL}", "current", "active", "files", "{channel-}", "resources")) } if wsl.IsWSL() { winHome, err := wsl.WindowsHome() if err == nil && winHome != "" { - // WSL. Data is stored under the Windows user's AppData folder. + // WSL. Windows Discord installs under the Windows user's AppData folder. // Example: `/mnt/c/Users/Username/AppData/Local/DiscordCanary`. - // Core: `/mnt/c/Users/Username/AppData/Local/DiscordCanary/app-1.0.9218/modules/discord_desktop_core-1/discord_desktop_core core.asar`. + // Resources: `/mnt/c/Users/Username/AppData/Local/DiscordCanary/app-1.0.9218/resources/app.asar`. paths = append(paths, filepath.Join(winHome, "AppData", "Local", "{CHANNEL}")) } } diff --git a/internal/discord/paths_test.go b/internal/discord/paths_test.go index f84c459..420c2a1 100644 --- a/internal/discord/paths_test.go +++ b/internal/discord/paths_test.go @@ -142,22 +142,49 @@ func TestGetChannel(t *testing.T) { expected: models.Stable, }, { - name: "Multiple Discord mentions (first wins)", + name: "Multiple Discord mentions (nearest wins)", path: filepath.Join("discordcanary", "discord", "modules"), - expected: models.Canary, + expected: models.Stable, // The nearest segment is "discord", which maps to Stable }, { name: "Empty path defaults to Stable", path: "", expected: models.Stable, }, + + // New injection + { + name: "macOS bundle name", + path: filepath.Join("/Applications", "Discord Canary.app", "Contents", "Resources"), + expected: models.Canary, + }, + { + name: "macOS stable bundle name", + path: filepath.Join("/Applications", "Discord.app", "Contents", "Resources"), + expected: models.Stable, + }, + { + name: "flatpak dashed canary dir", + path: filepath.Join("/var", "lib", "flatpak", "app", "com.discordapp.DiscordCanary", "current", "active", "files", "discord-canary", "resources"), + expected: models.Canary, + }, + { + name: "flatpak dashed ptb dir", + path: filepath.Join("/var", "lib", "flatpak", "app", "com.discordapp.DiscordPTB", "current", "active", "files", "discord-ptb", "resources"), + expected: models.PTB, + }, + { + name: "flatpak stable dir", + path: filepath.Join("/var", "lib", "flatpak", "app", "com.discordapp.Discord", "current", "active", "files", "discord", "resources"), + expected: models.Stable, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := GetChannel(tt.path) if result != tt.expected { - t.Errorf("GetChannel(%s) = %v (%s), expected %v (%s)", + t.Errorf("GetChannel(%q) = %v (%s), expected %v (%s)", tt.path, result, result.String(), tt.expected, tt.expected.String()) } }) @@ -211,12 +238,12 @@ func TestGetSuggestedPath(t *testing.T) { newCorePath := "/home/user/.config/discord/app-0.0.35/modules/discord_desktop_core-1/discord_desktop_core/core.asar" allDiscordInstalls[models.Stable] = []*DiscordInstall{ - {CorePath: oldCorePath, Version: "0.0.35"}, - {CorePath: "/usr/share/discord/0.0.34", Version: "0.0.34"}, + {ResourcesPath: oldCorePath, Version: "0.0.35"}, + {ResourcesPath: "/usr/share/discord/0.0.34", Version: "0.0.34"}, } allDiscordInstalls[models.Canary] = []*DiscordInstall{ - {CorePath: newCorePath, Version: "0.0.200"}, // New format + {ResourcesPath: newCorePath, Version: "0.0.200"}, // New format } // Test that it returns the first install (old format) @@ -261,14 +288,14 @@ func TestResolvePath(t *testing.T) { // Add a test install with new path format testInstall := &DiscordInstall{ - CorePath: "/home/user/.config/discord/app-1.0.0/modules/discord_desktop_core-1/discord_desktop_core/core.asar", - Channel: models.Stable, - Version: "1.0.0", + ResourcesPath: "/home/user/.config/discord/app-1.0.0/modules/discord_desktop_core-1/discord_desktop_core/core.asar", + Channel: models.Stable, + Version: "1.0.0", } allDiscordInstalls[models.Stable] = []*DiscordInstall{testInstall} // Test resolving existing path - result := ResolvePath(testInstall.CorePath) + result := ResolvePath(testInstall.ResourcesPath) if result != testInstall { t.Error("ResolvePath should return the existing install") } @@ -394,9 +421,9 @@ func TestSortInstalls(t *testing.T) { // Add unsorted installs - mix of old and new path formats allDiscordInstalls[models.Stable] = []*DiscordInstall{ - {CorePath: "/path1", Version: "0.0.34", Channel: models.Stable}, // Old format - {CorePath: "/home/user/.config/discord/app-0.0.36/modules/discord_desktop_core-1/discord_desktop_core/core.asar", Version: "0.0.36", Channel: models.Stable}, // New format - {CorePath: "/path3", Version: "0.0.35", Channel: models.Stable}, // Old format + {ResourcesPath: "/path1", Version: "0.0.34", Channel: models.Stable}, // Old format + {ResourcesPath: "/home/user/.config/discord/app-0.0.36/modules/discord_desktop_core-1/discord_desktop_core/core.asar", Version: "0.0.36", Channel: models.Stable}, // New format + {ResourcesPath: "/path3", Version: "0.0.35", Channel: models.Stable}, // Old format } // Sort them @@ -425,14 +452,14 @@ func TestSortInstalls_MultipleChannels(t *testing.T) { // Add unsorted installs for multiple channels - mix of old and new formats allDiscordInstalls[models.Stable] = []*DiscordInstall{ - {CorePath: "/stable1", Version: "1.0.0", Channel: models.Stable}, - {CorePath: "/home/user/.config/discord/app-1.0.2/modules/discord_desktop_core-1/discord_desktop_core/core.asar", Version: "1.0.2", Channel: models.Stable}, // New format + {ResourcesPath: "/stable1", Version: "1.0.0", Channel: models.Stable}, + {ResourcesPath: "/home/user/.config/discord/app-1.0.2/modules/discord_desktop_core-1/discord_desktop_core/core.asar", Version: "1.0.2", Channel: models.Stable}, // New format } allDiscordInstalls[models.Canary] = []*DiscordInstall{ - {CorePath: "/canary1", Version: "0.0.100", Channel: models.Canary}, - {CorePath: "/home/user/.config/discordcanary/app-0.0.150/modules/discord_desktop_core-1/discord_desktop_core/core.asar", Version: "0.0.150", Channel: models.Canary}, // New format - {CorePath: "/canary3", Version: "0.0.125", Channel: models.Canary}, + {ResourcesPath: "/canary1", Version: "0.0.100", Channel: models.Canary}, + {ResourcesPath: "/home/user/.config/discordcanary/app-0.0.150/modules/discord_desktop_core-1/discord_desktop_core/core.asar", Version: "0.0.150", Channel: models.Canary}, // New format + {ResourcesPath: "/canary3", Version: "0.0.125", Channel: models.Canary}, } // Sort them diff --git a/internal/discord/process.go b/internal/discord/process.go index 9694860..b78aeed 100644 --- a/internal/discord/process.go +++ b/internal/discord/process.go @@ -4,25 +4,55 @@ import ( "fmt" "os" "os/exec" + "time" "github.com/betterdiscord/cli/internal/output" "github.com/shirou/gopsutil/v3/process" ) -func (discord *DiscordInstall) restart() error { - exeName := discord.getFullExe() - - if running, _ := discord.isRunning(); !running { - output.Printf("โœ… %s is not running; skipping restart.\n", discord.Channel.Name()) - return nil +// killWaitTimeout bounds how long kill() waits for Discord's processes to fully +// exit after being signaled. Discord runs several processes; killing only +// signals termination, so we wait for them to actually die (releasing their lock +// on app.asar) before the caller touches it. +const killWaitTimeout = 10 * time.Second + +// stop terminates Discord if it is running. The new injection method modifies +// app.asar, which the running Discord process holds a lock on, so Discord must +// be stopped before inject/uninject can touch it. It returns the executable path +// of the killed process (captured before the kill, for a later start) and whether +// Discord was running. Flatpak/Snap relaunch via their own run commands and don't +// use the exe. +func (discord *DiscordInstall) stop() (exe string, wasRunning bool, err error) { + // If we can't even determine whether Discord is running, don't gamble on + // touching app.asar โ€” it may be locked. Fail with an actionable message + // rather than letting inject/uninject surface a confusing file error. + running, err := discord.isRunning() + if err != nil { + output.Printf("โŒ Unable to determine whether %s is running. Please close it and try again.\n", discord.Channel.Name()) + output.Printf(" %s\n", err.Error()) + return "", false, err + } + if !running { + output.Printf("โœ… %s is not running.\n", discord.Channel.Name()) + return "", false, nil } + // Capture the executable before killing โ€” afterward the process is gone. + exe = discord.getFullExe() + if err := discord.kill(); err != nil { - output.Printf("โŒ Unable to restart %s, please do so manually.\n", discord.Channel.Name()) + output.Printf("โŒ Unable to stop %s. Please close it and try again.\n", discord.Channel.Name()) output.Printf(" %s\n", err.Error()) - return err + return exe, true, err } + output.Printf("โœ… Stopped %s\n", discord.Channel.Name()) + return exe, true, nil +} + +// start launches Discord. exe is the executable path captured by stop() and is +// used for native installs; Flatpak/Snap launch via their run commands. +func (discord *DiscordInstall) start(exe string) error { // Determine command based on installation type var cmd *exec.Cmd if discord.IsFlatpak { @@ -30,12 +60,12 @@ func (discord *DiscordInstall) restart() error { } else if discord.IsSnap { cmd = exec.Command("snap", "run", discord.Channel.Exe()) } else { - // Use binary found in killing process for non-Flatpak/Snap installs - if exeName == "" { + // Use binary found while killing the process for non-Flatpak/Snap installs + if exe == "" { output.Printf("โŒ Unable to restart %s, please do so manually.\n", discord.Channel.Name()) return fmt.Errorf("could not determine executable path for %s", discord.Channel.Name()) } - cmd = exec.Command(exeName) + cmd = exec.Command(exe) } // Set working directory to user home @@ -54,9 +84,11 @@ func (discord *DiscordInstall) isRunning() (bool, error) { name := discord.Channel.Exe() processes, err := process.Processes() - // If we can't even list processes, bail out + // If we can't even list processes, bail out. Wrap the underlying error so + // callers (e.g. waitForExit) can surface the real cause instead of a bare + // "could not list processes". if err != nil { - return false, fmt.Errorf("could not list processes") + return false, fmt.Errorf("could not list processes: %w", err) } // Search for desired process(es) @@ -82,12 +114,15 @@ func (discord *DiscordInstall) kill() error { name := discord.Channel.Exe() processes, err := process.Processes() - // If we can't even list processes, bail out + // If we can't even list processes, bail out. Preserve the underlying error so + // a genuine enumeration failure is distinguishable from Discord still running + // (the caller's wait-for-exit surfaces the latter separately). if err != nil { - return fmt.Errorf("could not list processes") + return fmt.Errorf("could not list processes: %w", err) } // Search for desired process(es) + signaled := false for _, p := range processes { n, err := p.Name() @@ -104,11 +139,51 @@ func (discord *DiscordInstall) kill() error { if killErr != nil { return killErr } + signaled = true } } - // If we got here, everything was killed without error - return nil + if !signaled { + return nil + } + + // Kill() only signals termination; wait for the processes to actually exit so + // their lock on app.asar is released before the caller modifies it. + return discord.waitForExit(killWaitTimeout) +} + +// waitForExit blocks until no process matching the channel's executable remains, +// or the timeout elapses. A transient enumeration error is treated as +// "not yet confirmed exited" and retried rather than failing outright. If the +// most recent check couldn't enumerate processes at all, the timeout surfaces +// that underlying error instead of a misleading "did not exit" โ€” otherwise a +// persistent enumeration failure would send users chasing a lock that may not +// exist. +func (discord *DiscordInstall) waitForExit(timeout time.Duration) error { + deadline := time.Now().Add(timeout) + var lastErr error + for { + running, err := discord.isRunning() + switch { + case err != nil: + // Couldn't confirm state this round; remember why in case we time out + // with the failure still unresolved. + lastErr = err + case !running: + return nil + default: + // Clean read that still shows Discord running: the process, not + // enumeration, is the holdup โ€” clear any stale earlier error. + lastErr = nil + } + if time.Now().After(deadline) { + if lastErr != nil { + return fmt.Errorf("could not confirm %s exited within %s: %w", discord.Channel.Name(), timeout, lastErr) + } + return fmt.Errorf("%s did not exit within %s", discord.Channel.Name(), timeout) + } + time.Sleep(150 * time.Millisecond) + } } func (discord *DiscordInstall) getFullExe() string { diff --git a/internal/models/options.go b/internal/models/options.go new file mode 100644 index 0000000..8a6469c --- /dev/null +++ b/internal/models/options.go @@ -0,0 +1,20 @@ +package models + +type InstallOptions struct { + RestartDiscord bool `json:"restartDiscord"` + UseDevBuild bool `json:"useDevBuild"` +} + +type RepairOptions struct { + DisablePlugins bool `json:"disablePlugins"` + DisableThemes bool `json:"disableThemes"` + ClearCustomCSS bool `json:"clearCustomCSS"` + ClearWebpackCache bool `json:"clearWebpackCache"` + ClearAddonStoreCache bool `json:"clearAddonStoreCache"` + ResetSettings bool `json:"resetSettings"` +} + +type UninstallOptions struct { + FullUninstall bool `json:"fullUninstall"` + RestartDiscord bool `json:"restartDiscord"` +} diff --git a/internal/utils/strings.go b/internal/utils/strings.go index 05d1c64..c97814e 100644 --- a/internal/utils/strings.go +++ b/internal/utils/strings.go @@ -1,9 +1,82 @@ package utils -import "net/url" +import ( + "fmt" + "net/url" + "strings" +) // IsURL checks if a string is a valid URL func IsURL(input string) bool { parsed, err := url.Parse(input) return err == nil && parsed.Scheme != "" && parsed.Host != "" } + +// FormatVersion normalizes a version string with a single leading 'v'. +func FormatVersion(version string) string { + trimmed := strings.TrimSpace(version) + trimmed = strings.TrimPrefix(trimmed, "v") + if trimmed == "" { + return "v0.0.0" + } + return "v" + trimmed +} + +// CompareVersions compares two semantic versions (e.g., "1.0.156" vs "1.0.157") +// Returns -1 if v1 < v2, 0 if equal, 1 if v1 > v2 +func CompareVersions(v1, v2 string) int { + // Strip 'v' prefix if present + + if len(v1) > 0 && v1[0] == 'v' { + v1 = v1[1:] + } + if len(v2) > 0 && v2[0] == 'v' { + v2 = v2[1:] + } + + // Parse into version parts + parts1 := SplitVersion(v1) + parts2 := SplitVersion(v2) + + // Compare each part + maxLen := max(len(parts2), len(parts1)) + + for i := range maxLen { + var p1, p2 int + + if i < len(parts1) { + fmt.Sscanf(parts1[i], "%d", &p1) + } + if i < len(parts2) { + fmt.Sscanf(parts2[i], "%d", &p2) + } + + if p1 < p2 { + return -1 + } else if p1 > p2 { + return 1 + } + } + + return 0 +} + +// SplitVersion splits a version string into parts (e.g., "1.0.156" -> ["1", "0", "156"]) +func SplitVersion(v string) []string { + var parts []string + var current string + for i := 0; i < len(v); i++ { + if v[i] == '.' { + if current != "" { + parts = append(parts, current) + current = "" + } + } else if v[i] >= '0' && v[i] <= '9' { + current += string(v[i]) + } + } + if current != "" { + parts = append(parts, current) + } + return parts +} diff --git a/internal/utils/strings_test.go b/internal/utils/strings_test.go new file mode 100644 index 0000000..0deb2ae --- /dev/null +++ b/internal/utils/strings_test.go @@ -0,0 +1,135 @@ +package utils + +import ( + "reflect" + "testing" +) + +func TestFormatVersion(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "Trims and preserves leading v", + input: " v1.2.3 ", + expected: "v1.2.3", + }, + { + name: "Adds leading v", + input: "1.2.3", + expected: "v1.2.3", + }, + { + name: "Keeps v0.0.0", + input: "v0.0.0", + expected: "v0.0.0", + }, + { + name: "Empty defaults", + input: "", + expected: "v0.0.0", + }, + { + name: "Whitespace defaults", + input: " ", + expected: "v0.0.0", + }, + { + name: "Bare v defaults", + input: "v", + expected: "v0.0.0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := FormatVersion(tt.input) + if result != tt.expected { + t.Errorf("FormatVersion(%q) = %q, expected %q", tt.input, result, tt.expected) + } + }) + } +} + +func TestSplitVersion(t *testing.T) { + tests := []struct { + name string + input string + expected []string + }{ + { + name: "Standard version", + input: "1.0.156", + expected: []string{"1", "0", "156"}, + }, + { + name: "Skips empty segments", + input: "1..2", + expected: []string{"1", "2"}, + }, + { + name: "Ignores non-digits", + input: "v1.2.3-beta", + expected: []string{"1", "2", "3"}, + }, + { + name: "No digits", + input: "beta", + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := SplitVersion(tt.input) + if !reflect.DeepEqual(result, tt.expected) { + t.Errorf("SplitVersion(%q) = %v, expected %v", tt.input, result, tt.expected) + } + }) + } +} + +func TestCompareVersions(t *testing.T) { + tests := []struct { + name string + v1 string + v2 string + expected int + }{ + { + name: "Equal with v prefix", + v1: "v1.2.3", + v2: "1.2.3", + expected: 0, + }, + { + name: "Less than", + v1: "1.2.3", + v2: "1.2.4", + expected: -1, + }, + { + name: "Greater than", + v1: "2.0.0", + v2: "1.9.9", + expected: 1, + }, + { + name: "Missing parts default to zero", + v1: "1.2", + v2: "1.2.0", + expected: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := CompareVersions(tt.v1, tt.v2) + if result != tt.expected { + t.Errorf("CompareVersions(%q, %q) = %d, expected %d", tt.v1, tt.v2, result, tt.expected) + } + }) + } +} diff --git a/internal/wsl/wsl_test.go b/internal/wsl/wsl_test.go new file mode 100644 index 0000000..e347bd0 --- /dev/null +++ b/internal/wsl/wsl_test.go @@ -0,0 +1,50 @@ +package wsl + +import ( + "strings" + "sync" + "testing" +) + +func resetWSLInfo() { + once = sync.Once{} + info = nil +} + +func TestInfo_NotWSLWhenNoSignals(t *testing.T) { + t.Setenv("WSL_DISTRO_NAME", "") + t.Setenv("WSL_INTEROP", "") + resetWSLInfo() + + info := Info() + if strings.Contains(info.KernelVersion, "microsoft") { + if !info.IsWSL { + t.Fatalf("Expected IsWSL true when kernel indicates WSL") + } + } else if info.IsWSL { + t.Fatalf("Expected IsWSL false when no WSL signals are set") + } + if info.DistroName != "" { + t.Fatalf("Expected empty DistroName, got %q", info.DistroName) + } + if info.InteropPath != "" { + t.Fatalf("Expected empty InteropPath, got %q", info.InteropPath) + } +} + +func TestInfo_WSLDistroName(t *testing.T) { + t.Setenv("WSL_DISTRO_NAME", "Ubuntu") + t.Setenv("WSL_INTEROP", "interop-path") + resetWSLInfo() + + info := Info() + if !info.IsWSL { + t.Fatalf("Expected IsWSL true when WSL_DISTRO_NAME is set") + } + if info.DistroName != "Ubuntu" { + t.Fatalf("Expected DistroName Ubuntu, got %q", info.DistroName) + } + if info.InteropPath != "interop-path" { + t.Fatalf("Expected InteropPath interop-path, got %q", info.InteropPath) + } +}