From c67be72869f6c3855a53d3b64d3fa3ec308d58a1 Mon Sep 17 00:00:00 2001 From: Joren Date: Tue, 21 Apr 2026 19:04:33 +0200 Subject: [PATCH] harden search parsing and qobuz refresh validation --- cmd/rip/main.go | 19 ++++++++++++++ cmd/rip/main_test.go | 35 ++++++++++++++++++++++++++ internal/download/downloader.go | 6 +++++ internal/provider/qobuz/client.go | 16 ++++++++++-- internal/provider/qobuz/client_test.go | 12 +++++++++ 5 files changed, 86 insertions(+), 2 deletions(-) diff --git a/cmd/rip/main.go b/cmd/rip/main.go index 8c14a0a..8d1d251 100644 --- a/cmd/rip/main.go +++ b/cmd/rip/main.go @@ -13,6 +13,7 @@ import ( "net/url" "os" "os/exec" + "path/filepath" "regexp" "runtime" "strconv" @@ -1995,6 +1996,12 @@ func parseSearchArgs(args []string, defaultLimit int) (searchOptions, error) { first := false outputFile := "" for i := 0; i < len(args); i++ { + if args[i] == "--" { + if i+1 < len(args) { + parts = append(parts, args[i+1:]...) + } + break + } switch args[i] { case "--force", "--ignore-db": ignoreDB = true @@ -2010,6 +2017,9 @@ func parseSearchArgs(args []string, defaultLimit int) (searchOptions, error) { return searchOptions{}, fmt.Errorf("--output-file requires a path") } outputFile = strings.TrimSpace(args[i+1]) + if outputFile == "" { + return searchOptions{}, fmt.Errorf("--output-file requires a non-empty path") + } i++ continue case "--num-results": @@ -2036,6 +2046,9 @@ func parseSearchArgs(args []string, defaultLimit int) (searchOptions, error) { i++ continue } + if strings.HasPrefix(args[i], "-") { + return searchOptions{}, fmt.Errorf("unknown option %q", args[i]) + } parts = append(parts, args[i]) } return searchOptions{ @@ -2196,6 +2209,12 @@ func writeSearchResultsToFile(source, mediaType string, results []searchResult, if err != nil { return err } + dir := filepath.Dir(path) + if dir != "" && dir != "." { + if err = os.MkdirAll(dir, 0o755); err != nil { + return err + } + } return os.WriteFile(path, b, 0o644) } diff --git a/cmd/rip/main_test.go b/cmd/rip/main_test.go index 24e61e4..eb020e3 100644 --- a/cmd/rip/main_test.go +++ b/cmd/rip/main_test.go @@ -2,6 +2,9 @@ package main import ( "errors" + "os" + "path/filepath" + "strings" "testing" ) @@ -198,6 +201,38 @@ func TestParseSearchArgsAllowsFirstAndOutputFileButCallerCanReject(t *testing.T) } } +func TestParseSearchArgsRejectsUnknownOption(t *testing.T) { + _, err := parseSearchArgs([]string{"query", "--bogus"}, 20) + if err == nil || !strings.Contains(err.Error(), "unknown option") { + t.Fatalf("expected unknown option error, got %v", err) + } +} + +func TestParseSearchArgsSupportsDoubleDashTerminator(t *testing.T) { + opts, err := parseSearchArgs([]string{"--limit", "10", "--", "--weird", "track"}, 20) + if err != nil { + t.Fatalf("parseSearchArgs() error = %v", err) + } + if opts.limit != 10 { + t.Fatalf("limit = %d, want 10", opts.limit) + } + if opts.query != "--weird track" { + t.Fatalf("query = %q, want %q", opts.query, "--weird track") + } +} + +func TestWriteSearchResultsToFileCreatesParentDirectory(t *testing.T) { + tmp := t.TempDir() + out := filepath.Join(tmp, "nested", "search", "results.json") + results := []searchResult{{ID: "1", Title: "Dreams"}} + if err := writeSearchResultsToFile("qobuz", "track", results, out); err != nil { + t.Fatalf("writeSearchResultsToFile() error = %v", err) + } + if _, err := os.Stat(out); err != nil { + t.Fatalf("expected output file, stat error = %v", err) + } +} + func TestErrorWithActionableHintForSSL(t *testing.T) { err := errors.New("x509: certificate signed by unknown authority") msg := errorWithActionableHint(err, globalOptions{}) diff --git a/internal/download/downloader.go b/internal/download/downloader.go index 82b6f60..06ed1ff 100644 --- a/internal/download/downloader.go +++ b/internal/download/downloader.go @@ -147,6 +147,9 @@ func (d *Downloader) FileDeezerEncrypted(ctx context.Context, sourceURL, outputP if resp.ContentLength > 0 && totalRead != resp.ContentLength { return io.ErrUnexpectedEOF } + if err = out.Sync(); err != nil { + return err + } success = true return nil } @@ -229,6 +232,9 @@ func (d *Downloader) file(ctx context.Context, sourceURL, outputPath string, all if resp.ContentLength > 0 && totalWritten != resp.ContentLength { return io.ErrUnexpectedEOF } + if err = out.Sync(); err != nil { + return err + } } else { written, copyErr := io.Copy(out, reader) if copyErr != nil { diff --git a/internal/provider/qobuz/client.go b/internal/provider/qobuz/client.go index aabbdf6..d305d32 100644 --- a/internal/provider/qobuz/client.go +++ b/internal/provider/qobuz/client.go @@ -146,9 +146,21 @@ func (c *Client) refreshAppCredentials(ctx context.Context, q *config.QobuzConfi return err } q.AppID = strings.TrimSpace(appID) - q.Secrets = append([]string(nil), secrets...) + if q.AppID == "" { + return errors.New("qobuz app credential refresh returned empty app_id") + } + clean := make([]string, 0, len(secrets)) + for _, s := range secrets { + if v := strings.TrimSpace(s); v != "" { + clean = append(clean, v) + } + } + if len(clean) == 0 { + return errors.New("qobuz app credential refresh returned no secrets") + } + q.Secrets = append([]string(nil), clean...) c.cfg.File.Qobuz.AppID = q.AppID - c.cfg.File.Qobuz.Secrets = append([]string(nil), secrets...) + c.cfg.File.Qobuz.Secrets = append([]string(nil), clean...) _ = c.cfg.SaveFile() return nil } diff --git a/internal/provider/qobuz/client_test.go b/internal/provider/qobuz/client_test.go index b1a6497..d430d2b 100644 --- a/internal/provider/qobuz/client_test.go +++ b/internal/provider/qobuz/client_test.go @@ -373,3 +373,15 @@ func qobuzSecretSig(requestTS, secret string) string { hash := md5.Sum([]byte(raw)) return hex.EncodeToString(hash[:]) } + +func TestRefreshAppCredentialsRejectsEmptyData(t *testing.T) { + d := config.DefaultConfigData() + c := New(&config.Config{File: d, Session: d}) + c.fetchCfg = func(context.Context) (string, []string, error) { + return "", []string{" "}, nil + } + err := c.refreshAppCredentials(context.Background(), &c.cfg.Session.Qobuz) + if err == nil { + t.Fatalf("expected error for empty refreshed app credentials") + } +}