refactor: Enhance destination path validation and sanitization in download functions; add support for episode switching without updater

Author: alvarorichard

internal/api/allanime_smart.go

@@ -79,23 +79,28 @@ func DownloadAllAnimeSmartRange(anime *models.Anime, startEp, endEp int, quality
 
 // smartDownload chooses the best method to download AllAnime links (HLS/hosters)
 func smartDownload(url, dest string) error {
+	// Sanitize and validate destination path under the downloads root
+	safeDest, err := sanitizeSmartDest(dest)
+	if err != nil {
+		return err
+	}
 	// Ensure destination directory exists
-	if err := os.MkdirAll(filepath.Dir(dest), 0700); err != nil {
+	if err := os.MkdirAll(filepath.Dir(safeDest), 0700); err != nil {
 		return err
 	}
 
 	// Use yt-dlp for HLS/known hosters
 	if shouldUseYtDlp(url) {
 		ctx := context.Background()
 		ytdlp.MustInstall(ctx, nil)
-		dl := ytdlp.New().Output(dest)
+		dl := ytdlp.New().Output(safeDest)
 		_, err := dl.Run(ctx, url)
 		if err != nil {
 			return fmt.Errorf("yt-dlp failed: %w", err)
 		}
 		// Verify
-		if st, err := os.Stat(dest); err != nil || st.Size() < 1024 {
-			return fmt.Errorf("download verification failed for %s", dest)
+		if st, err := os.Stat(safeDest); err != nil || st.Size() < 1024 {
+			return fmt.Errorf("download verification failed for %s", safeDest)
 		}
 		return nil
 	}
@@ -114,7 +119,8 @@ func smartDownload(url, dest string) error {
 	if resp.StatusCode != http.StatusOK {
 		return fmt.Errorf("bad status: %s", resp.Status)
 	}
-	out, err := os.Create(dest)
+	// #nosec G304: path validated by sanitizeSmartDest to remain within the GoAnime downloads root
+	out, err := os.Create(safeDest)
 	if err != nil {
 		return err
 	}
@@ -191,6 +197,38 @@ func sanitizeSmart(name string) string {
 	return name
 }
 
+// sanitizeSmartDest ensures destination path is within the GoAnime downloads root under the user's home
+func sanitizeSmartDest(p string) (string, error) {
+	if strings.TrimSpace(p) == "" {
+		return "", fmt.Errorf("empty destination path")
+	}
+	if strings.HasPrefix(p, "-") || strings.ContainsAny(p, "\x00\n\r") {
+		return "", fmt.Errorf("invalid destination path")
+	}
+	cleaned := filepath.Clean(p)
+	home, err := os.UserHomeDir()
+	if err != nil {
+		return "", err
+	}
+	root := filepath.Join(home, ".local", "goanime", "downloads", "anime")
+	absRoot, err := filepath.Abs(root)
+	if err != nil {
+		return "", err
+	}
+	absFile, err := filepath.Abs(cleaned)
+	if err != nil {
+		return "", err
+	}
+	rel, err := filepath.Rel(absRoot, absFile)
+	if err != nil {
+		return "", err
+	}
+	if strings.HasPrefix(rel, "..") {
+		return "", fmt.Errorf("destination escapes downloads root: %s", cleaned)
+	}
+	return absFile, nil
+}
+
 // Helpers to reduce complexity
 
 // validateSmartRangeInputs ensures correct source and quality defaulting

internal/player/player.go

@@ -27,6 +27,9 @@ import (
 	"github.com/pkg/errors"
 )
 
+// lastAnimeURL stores the most recent anime URL/ID to support navigation when no updater is present
+var lastAnimeURL string
+
 const (
 	padding = 2
 )
@@ -332,6 +335,8 @@ func HandleDownloadAndPlay(
 	animeMalID int,
 	updater *discord.RichPresenceUpdater,
 ) error {
+	// Persist the anime URL/ID to aid episode switching when updater is nil (e.g., Discord disabled)
+	lastAnimeURL = animeURL
 	downloadOption := askForDownload()
 	switch downloadOption {
 	case 1:

internal/player/playvideo.go

@@ -674,6 +674,15 @@ func switchEpisode(newIndex int, episodes []models.Episode, anilistID int, updat
 		anime = updater.GetAnime()
 	}
 
+	// If no updater/anime context, try to synthesize from lastAnimeURL
+	if anime == nil && lastAnimeURL != "" {
+		guessedSource := ""
+		if (len(lastAnimeURL) < 30 && !strings.Contains(lastAnimeURL, "http")) || strings.Contains(lastAnimeURL, "allanime") {
+			guessedSource = "AllAnime"
+		}
+		anime = &models.Anime{URL: lastAnimeURL, Source: guessedSource}
+	}
+
 	targetURL, err := GetVideoURLForEpisodeEnhanced(&target, anime)
 	if err != nil {
 		return fmt.Errorf("failed to get video URL: %w", err)

internal/player/scraper.go

@@ -259,6 +259,40 @@ func GetVideoURLForEpisode(episodeURL string) (string, error) {
 
 // GetVideoURLForEpisodeEnhanced gets the video URL using the enhanced API with AllAnime navigation support
 func GetVideoURLForEpisodeEnhanced(episode *models.Episode, anime *models.Anime) (string, error) {
+	// If we don't have anime context, decide safely how to resolve
+	if anime == nil {
+		// If it's a normal HTTP URL, use legacy extraction
+		if strings.Contains(episode.URL, "http") {
+			if util.IsDebug {
+				util.Debugf("No anime context; using legacy extraction for HTTP URL, episode %s", episode.Number)
+			}
+			return GetVideoURLForEpisode(episode.URL)
+		}
+
+		// If episode.URL looks like an AllAnime ID, synthesize minimal anime context
+		if isLikelyAllAnimeID(episode.URL) {
+			if util.IsDebug {
+				util.Debugf("No anime context; detected AllAnime ID '%s'. Using enhanced API with synthetic anime context.", episode.URL)
+			}
+			tmpAnime := &models.Anime{
+				URL:    episode.URL,
+				Source: "AllAnime",
+				Name:   "[AllAnime]",
+			}
+			// Ensure episode number is set
+			if episode.Number == "" && episode.Num > 0 {
+				episode.Number = fmt.Sprintf("%d", episode.Num)
+			}
+			if episode.Number == "" {
+				episode.Number = "1"
+			}
+			return api.GetEpisodeStreamURLEnhanced(episode, tmpAnime, util.GlobalQuality)
+		}
+
+		// If it's likely just an episode number without anime context, we cannot resolve via enhanced API
+		return "", fmt.Errorf("cannot resolve stream without anime context for episode %s; missing anime identifier", episode.Number)
+	}
+
 	// Try AllAnime enhanced navigation first if applicable
 	if isAllAnimeSourcePlayer(anime) {
 		streamURL, err := api.GetEpisodeStreamURLEnhanced(episode, anime, util.GlobalQuality)
@@ -283,6 +317,9 @@ func GetVideoURLForEpisodeEnhanced(episode *models.Episode, anime *models.Anime)
 
 // Helper function to check if anime is from AllAnime source (player module)
 func isAllAnimeSourcePlayer(anime *models.Anime) bool {
+	if anime == nil {
+		return false
+	}
 	if anime.Source == "AllAnime" {
 		return true
 	}
@@ -300,6 +337,32 @@ func isAllAnimeSourcePlayer(anime *models.Anime) bool {
 	return false
 }
 
+// Helper: detect if a string is purely numeric (e.g., "12" or "12.5")
+func isNumericString(s string) bool {
+	if s == "" {
+		return false
+	}
+	re := regexp.MustCompile(`^\d+(?:\.\d+)?$`)
+	return re.MatchString(s)
+}
+
+// Helper: detect if the value looks like an AllAnime ID (short, non-HTTP, alphanumeric with letters)
+func isLikelyAllAnimeID(s string) bool {
+	if strings.Contains(s, "http") {
+		return false
+	}
+	if isNumericString(s) {
+		return false
+	}
+	// Typical AllAnime IDs are short-ish alphanumeric strings
+	if len(s) >= 6 && len(s) < 30 {
+		// Must contain at least one letter
+		re := regexp.MustCompile(`[A-Za-z]`)
+		return re.MatchString(s)
+	}
+	return false
+}
+
 func extractVideoURL(url string) (string, error) {
 	if util.IsDebug {
 		util.Debugf("Extracting video URL from page: %s", url)