From 874d6ffe70d47d358142be1694422913e213c582 Mon Sep 17 00:00:00 2001 From: sim Date: Tue, 15 Sep 2026 12:53:58 +0800 Subject: [PATCH 1/3] feat(zapscript): trust a ZapLink the account vouches for as the user's own - A link service may answer a credentialed request with a header saying the linked account owns the card or deck the link names. The answer is honoured only from a host this device sent its own credential to, and absent means not owned. - A link body the account vouched for runs trusted, like a card the user wrote; any other link body runs untrusted, as before. A cached copy of a deck opened through such a link runs trusted for that open without becoming an owned, synced deck. --- pkg/platforms/platforms.go | 6 +- pkg/zapscript/commands.go | 16 +++++- pkg/zapscript/commands_test.go | 46 +++++++++++++++ pkg/zapscript/playlist_deck.go | 5 +- pkg/zapscript/playlist_deck_test.go | 6 ++ pkg/zapscript/virtual_launchables_test.go | 3 +- pkg/zapscript/zaplinks.go | 70 ++++++++++++++++------- pkg/zapscript/zaplinks_test.go | 38 +++++++++++- 8 files changed, 162 insertions(+), 28 deletions(-) diff --git a/pkg/platforms/platforms.go b/pkg/platforms/platforms.go index 65131e71f..58abf2dc8 100644 --- a/pkg/platforms/platforms.go +++ b/pkg/platforms/platforms.go @@ -222,7 +222,11 @@ type CmdEnv struct { // in the background as the deck opens. It returns at once and never // blocks the open; nil when no sync is configured. RefreshOwnedDeck func(ctx context.Context, deckID string) - PlaybackManager audio.PlaybackManager + // TrustedDeckID names a deck whose link the linked account vouched for + // as the user's own for this command, so a cached copy of it opens + // trusted this once. Empty otherwise. + TrustedDeckID string + PlaybackManager audio.PlaybackManager // LauncherCache resolves the launcher behind the active media. It holds // launchers the platform cannot build itself, so it is the only complete // source; never resolve a launcher ID from Platform.Launchers alone. diff --git a/pkg/zapscript/commands.go b/pkg/zapscript/commands.go index bbca5bd61..6be8d3cd1 100644 --- a/pkg/zapscript/commands.go +++ b/pkg/zapscript/commands.go @@ -40,6 +40,7 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/boolutil" "github.com/ZaparooProject/zaparoo-core/v2/pkg/platforms" "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/decks" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/playlists" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/state" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/tokens" @@ -494,9 +495,11 @@ func RunCommand( // substitute in server-fetched ZapScript would bypass the remote // operation allowlist entirely. linkValue := "" + linkOwned := false + trustedDeckID := "" if token.Source != tokens.SourceRemote { var linkErr error - linkValue, linkErr = checkZapLink(cfg, pl, db, cmd) + linkValue, linkOwned, linkErr = checkZapLink(cfg, pl, db, cmd) if linkErr != nil { return platforms.CmdResult{}, fmt.Errorf("zap link error: %w", linkErr) } @@ -523,7 +526,15 @@ func RunCommand( } cmd = script.Cmds[0] - unsafe = true + // A link the account vouched for as the user's own card or deck runs + // trusted, the way a card the user wrote does; any other link body + // runs untrusted. + unsafe = !linkOwned + if linkOwned && cmd.Name == zapscript.ZapScriptCmdPlaylistOpen && len(cmd.Args) == 1 { + if deckID, isDeck := decks.ParseDeckURI(cmd.Args[0]); isDeck { + trustedDeckID = deckID + } + } } for i, arg := range cmd.Args { @@ -568,6 +579,7 @@ func RunCommand( PrepareMediaLaunch: opts.PrepareMediaLaunch, BeforeExit: opts.BeforeExit, RefreshOwnedDeck: opts.RefreshOwnedDeck, + TrustedDeckID: trustedDeckID, PlaybackManager: opts.PlaybackManager, LauncherCache: helpers.GlobalLauncherCache, UI: opts.UI, diff --git a/pkg/zapscript/commands_test.go b/pkg/zapscript/commands_test.go index fa025232c..473853846 100644 --- a/pkg/zapscript/commands_test.go +++ b/pkg/zapscript/commands_test.go @@ -23,8 +23,11 @@ import ( "bytes" "encoding/json" "fmt" + "net/http" + "net/http/httptest" "os" "path/filepath" + "sync/atomic" "testing" "github.com/ZaparooProject/go-zapscript" @@ -1000,6 +1003,49 @@ func TestRunCommandCountsZapLinkExpansionInTotalCommands(t *testing.T) { // TestRunCommandAppliesZapLinkForNonRemoteSource pins that every other // token source still goes through ZapLink resolution as before. The // remote-source skip in RunCommand must not become a blanket skip. +// TestRunCommandZapLinkTrustFollowsTheOwnedAnswer pins that a link body runs +// untrusted unless a credentialed link service vouched for it as the user's +// own, in which case it runs trusted like a card the user wrote. +func TestRunCommandZapLinkTrustFollowsTheOwnedAnswer(t *testing.T) { + // No t.Parallel(): the auth config is global. + var sendHeader atomic.Bool + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", MIMEZaparooZapScript) + if sendHeader.Load() { + w.Header().Set(HeaderZaparooOwned, "1") + } + // A command skipped by its own condition answers with the trust it + // would have run under, without running anything. + _, _ = w.Write([]byte("**input.keyboard:a?when=false")) + })) + defer server.Close() + config.SetAuthCfgForTesting(map[string]config.CredentialEntry{ + config.RemoteAuthLookupURL(server.URL): {Bearer: "zpd1_test"}, + }) + t.Cleanup(config.ClearAuthCfgForTesting) + + mockUserDB := &testhelpers.MockUserDBI{} + mockUserDB.On("GetZapLinkHost", server.URL).Return(true, true, nil) + mockUserDB.On("UpdateZapLinkCache", server.URL+"/c1", mock.Anything).Return(nil) + db := &database.Database{UserDB: mockUserDB} + mockPlatform := mocks.NewMockPlatform() + mockPlatform.On("ID").Return("test") + + run := func() platforms.CmdResult { + result, err := RunCommand( + t.Context(), mockPlatform, &config.Instance{}, playlists.PlaylistController{}, + tokens.Token{Source: tokens.SourceReader}, + zapscript.Command{Name: "launch", Args: []string{server.URL + "/c1"}}, + 1, 0, db, &RunCommandOptions{}, &zapscript.ArgExprEnv{}, + ) + require.NoError(t, err) + return result + } + assert.True(t, run().Unsafe, "a link body runs untrusted by default") + sendHeader.Store(true) + assert.False(t, run().Unsafe, "a link the account vouched for runs trusted") +} + func TestRunCommandAppliesZapLinkForNonRemoteSource(t *testing.T) { t.Parallel() diff --git a/pkg/zapscript/playlist_deck.go b/pkg/zapscript/playlist_deck.go index f321afc7b..e7f6b1c18 100644 --- a/pkg/zapscript/playlist_deck.go +++ b/pkg/zapscript/playlist_deck.go @@ -107,8 +107,9 @@ func loadDeckPlaylist( pls := playlists.NewPlaylist(decks.PlaylistID(deck.DeckID), deck.Name, items) // Only the user's own decks are trusted. A cached copy of somebody // else's deck runs its items the way its ZapLink would, whatever script - // opened it. - pls.Unsafe = !deck.Owned + // opened it, unless the account vouched for this very link as the + // user's own deck. + pls.Unsafe = !deck.Owned && env.TrustedDeckID != deck.DeckID slot, slotErr := mediaslot.Normalize(env.Cmd.AdvArgs.Get(zapscript.KeySlot)) if slotErr != nil { return nil, fmt.Errorf("normalize media slot: %w", slotErr) diff --git a/pkg/zapscript/playlist_deck_test.go b/pkg/zapscript/playlist_deck_test.go index 4fab632c6..d86a2c2fc 100644 --- a/pkg/zapscript/playlist_deck_test.go +++ b/pkg/zapscript/playlist_deck_test.go @@ -103,6 +103,12 @@ func TestCmdPlaylistLoad_NotOwnedDeckIsUntrusted(t *testing.T) { require.NoError(t, err) pls := <-queue assert.True(t, pls.Unsafe, "a cached copy of somebody else's deck runs its items untrusted") + + env.TrustedDeckID = "0123456789ab" + _, err = cmdPlaylistLoad(newPlaylistTestPlatform(), env) + require.NoError(t, err) + pls = <-queue + assert.False(t, pls.Unsafe, "unless the account vouched for this link as the user's own deck") } func TestCmdPlaylistLoad_DeckNotFound(t *testing.T) { diff --git a/pkg/zapscript/virtual_launchables_test.go b/pkg/zapscript/virtual_launchables_test.go index a02b42731..2e0e24063 100644 --- a/pkg/zapscript/virtual_launchables_test.go +++ b/pkg/zapscript/virtual_launchables_test.go @@ -29,8 +29,9 @@ import ( func TestCheckZapLink_IgnoresVirtualPathSchemes(t *testing.T) { cmd := gozapscript.Command{Args: []string{"zaparoo://mfsg22lomvzxiyi/Chess"}} - target, err := checkZapLink(nil, nil, nil, cmd) + target, owned, err := checkZapLink(nil, nil, nil, cmd) require.NoError(t, err) assert.Empty(t, target) + assert.False(t, owned) } diff --git a/pkg/zapscript/zaplinks.go b/pkg/zapscript/zaplinks.go index d2081931f..9a333cff9 100644 --- a/pkg/zapscript/zaplinks.go +++ b/pkg/zapscript/zaplinks.go @@ -278,6 +278,12 @@ func isKnownZapLinkHost(link string, db *database.Database) bool { return supported } +// HeaderZaparooOwned is set by a link service on a response to a request +// that carried the device's credential, when the account behind that +// credential owns the card or deck the link names. It never appears in the +// shared body every requester gets. +const HeaderZaparooOwned = "X-Zaparoo-Owned" + func getRemoteZapScript(urlStr, platform string) ([]byte, error) { return getRemoteZapScriptContext(context.Background(), urlStr, platform) } @@ -285,25 +291,35 @@ func getRemoteZapScript(urlStr, platform string) ([]byte, error) { // getRemoteZapScriptContext fetches a ZapLink body within ctx, bounded by the // same ten-second ceiling a plain fetch uses. func getRemoteZapScriptContext(parent context.Context, urlStr, platform string) ([]byte, error) { + body, _, err := getRemoteZapScriptOwned(parent, urlStr, platform) + return body, err +} + +// getRemoteZapScriptOwned fetches a ZapLink body and reports whether the +// link service said the linked account owns what the link names. The answer +// is trusted only from a host this device sent its own credential to: a +// host that got no credential cannot know who asked, so its claim is +// ignored. Absent means not owned. +func getRemoteZapScriptOwned(parent context.Context, urlStr, platform string) (body []byte, owned bool, err error) { ctx, cancel := context.WithTimeout(parent, 10*time.Second) defer cancel() req, err := http.NewRequestWithContext(ctx, http.MethodGet, urlStr, http.NoBody) if err != nil { - return nil, fmt.Errorf("failed to create request for '%s': %w", urlStr, err) + return nil, false, fmt.Errorf("failed to create request for '%s': %w", urlStr, err) } if validationErr := validateZapLinkURL(req.URL); validationErr != nil { - return nil, validationErr + return nil, false, validationErr } setZapLinkHeaders(req, platform) req.Header.Set("Accept", strings.Join(AcceptedMimeTypes, ", ")) resp, err := currentZapFetchClient().Do(req) //nolint:gosec // G704: URL from ZapLink resolution if err != nil { - return nil, fmt.Errorf("failed to fetch zapscript from '%s': %w", urlStr, err) + return nil, false, fmt.Errorf("failed to fetch zapscript from '%s': %w", urlStr, err) } if resp == nil { - return nil, errors.New("received nil response") + return nil, false, errors.New("received nil response") } defer func() { if closeErr := resp.Body.Close(); closeErr != nil { @@ -313,12 +329,12 @@ func getRemoteZapScriptContext(parent context.Context, urlStr, platform string) if resp.StatusCode != http.StatusOK { log.Debug().Msgf("status code: %d", resp.StatusCode) - return nil, errors.New("invalid status code") + return nil, false, errors.New("invalid status code") } contentType := resp.Header.Get("Content-Type") if contentType == "" { - return nil, errors.New("content type is empty") + return nil, false, errors.New("content type is empty") } content := "" @@ -330,21 +346,29 @@ func getRemoteZapScriptContext(parent context.Context, urlStr, platform string) } if content == "" { - return nil, errors.New("no valid content type") + return nil, false, errors.New("no valid content type") } - body, err := io.ReadAll(io.LimitReader(resp.Body, helpers.MaxResponseBodySize)) + body, err = io.ReadAll(io.LimitReader(resp.Body, helpers.MaxResponseBodySize)) if err != nil { - return nil, fmt.Errorf("error reading body: %w", err) + return nil, false, fmt.Errorf("error reading body: %w", err) } if content != MIMEZaparooZapScript { - return nil, errors.New("invalid content type") + return nil, false, errors.New("invalid content type") } - log.Debug().Int("size", len(body)).Msg("received zap link body") + owned = resp.Header.Get(HeaderZaparooOwned) == "1" && credentialSentTo(urlStr) + log.Debug().Int("size", len(body)).Bool("owned", owned).Msg("received zap link body") + + return body, owned, nil +} - return body, nil +// credentialSentTo reports whether the device attaches its own credential to +// requests for urlStr, the same lookup the request transport makes. +func credentialSentTo(urlStr string) bool { + entry := config.LookupAuth(config.GetAuthCfg(), urlStr) + return entry != nil && entry.Bearer != "" } // validateZapLinkURL enforces HTTPS except for localhost and literal @@ -439,34 +463,38 @@ func isOfflineError(err error) bool { return false } +// checkZapLink resolves a command whose first argument is a ZapLink into the +// script the link serves, and reports whether the link service said the +// linked account owns what the link names. A body served from the offline +// cache is never owned: nobody vouched for it this time. func checkZapLink( _ *config.Instance, pl platforms.Platform, db *database.Database, cmd zapscript.Command, -) (string, error) { +) (script string, owned bool, err error) { if len(cmd.Args) == 0 { - return "", nil + return "", false, nil } value := cmd.Args[0] if !isZapLink(value, db) { - return "", nil + return "", false, nil } platform := pl.ID() log.Info().Msgf("checking zap link: %s", value) - body, err := getRemoteZapScript(value, platform) + body, owned, err := getRemoteZapScriptOwned(context.Background(), value, platform) if isOfflineError(err) { cachedScript, cacheErr := db.UserDB.GetZapLinkCache(value) if cacheErr != nil { - return "", fmt.Errorf("failed to get zaplink cache for '%s': %w", value, cacheErr) + return "", false, fmt.Errorf("failed to get zaplink cache for '%s': %w", value, cacheErr) } if cachedScript != "" { - return cachedScript, nil + return cachedScript, false, nil } } if err != nil { - return "", err + return "", false, err } err = db.UserDB.UpdateZapLinkCache(value, string(body)) @@ -475,9 +503,9 @@ func checkZapLink( } if !helpers.MaybeJSON(body) { - return string(body), nil + return string(body), owned, nil } - return "", errors.New("zapscript JSON not supported") + return "", false, errors.New("zapscript JSON not supported") } // PreWarmZapLinkHosts pre-warms the DNS and TLS cache for known zaplink hosts. diff --git a/pkg/zapscript/zaplinks_test.go b/pkg/zapscript/zaplinks_test.go index e60445a10..becee366f 100644 --- a/pkg/zapscript/zaplinks_test.go +++ b/pkg/zapscript/zaplinks_test.go @@ -32,6 +32,7 @@ import ( "path/filepath" "runtime" "strings" + "sync/atomic" "testing" gozapscript "github.com/ZaparooProject/go-zapscript" @@ -840,14 +841,49 @@ func TestCheckZapLinkFetchesSupportedRemoteScript(t *testing.T) { platform := mocks.NewMockPlatform() platform.On("ID").Return("mister") - target, err := checkZapLink(nil, platform, db, gozapscript.Command{Args: []string{server.URL + "/token"}}) + target, owned, err := checkZapLink(nil, platform, db, gozapscript.Command{Args: []string{server.URL + "/token"}}) require.NoError(t, err) assert.Equal(t, expected, target) + assert.False(t, owned, "a host that got no credential cannot vouch for ownership") mockUserDB.AssertExpectations(t) platform.AssertExpectations(t) } +// TestGetRemoteZapScriptOwnedNeedsACredential pins the trust rule for the +// owned header: it is honoured only from a host this device sent its own +// credential to, and absent means not owned. +func TestGetRemoteZapScriptOwnedNeedsACredential(t *testing.T) { + // No t.Parallel(): the auth config is global. + var sendHeader atomic.Bool + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", MIMEZaparooZapScript) + if sendHeader.Load() { + w.Header().Set(HeaderZaparooOwned, "1") + } + _, _ = w.Write([]byte("**launch.system:snes")) + })) + defer server.Close() + + sendHeader.Store(true) + _, owned, err := getRemoteZapScriptOwned(context.Background(), server.URL+"/c1", "test") + require.NoError(t, err) + assert.False(t, owned, "no credential was sent, so the claim is ignored") + + config.SetAuthCfgForTesting(map[string]config.CredentialEntry{ + config.RemoteAuthLookupURL(server.URL): {Bearer: "zpd1_test"}, + }) + t.Cleanup(config.ClearAuthCfgForTesting) + _, owned, err = getRemoteZapScriptOwned(context.Background(), server.URL+"/c1", "test") + require.NoError(t, err) + assert.True(t, owned, "the credentialed host vouched for the link") + + sendHeader.Store(false) + _, owned, err = getRemoteZapScriptOwned(context.Background(), server.URL+"/c1", "test") + require.NoError(t, err) + assert.False(t, owned, "absent means not owned") +} + // FetchWellKnown Tests func TestFetchWellKnownRejectsInvalidBaseURL(t *testing.T) { From fa2a6a379816efec109b7cb2e8d95b8641db82e5 Mon Sep 17 00:00:00 2001 From: sim Date: Sat, 19 Sep 2026 14:39:42 +0800 Subject: [PATCH 2/3] fix(decks): keep ZapLink decks from any host and harden the owned answer - A ZapLink body that is a single playlist.open, playlist.play or playlist.load command carrying a JSON playlist is kept as a read-only deck, whichever host serves it and whatever the link or the playlist ID look like. The copy is known by the link it was fetched from and gets a deck ID minted on this device, so nothing a source serves can name, replace or stand in for a deck the user owns. This replaces recognising a deck by an official host list, a "d" path and a "ZON-" playlist prefix, which tied the feature to one service and never matched a real link. - A deck made on the device opens as playlist ID deck://; a deck kept from a link opens as the id its playlist was served with. An open deck's in-place refresh matches the deck the playlist was opened from, never the playlist ID, which any served playlist may choose freely. - At most 200 decks are kept from links; the one fetched longest ago makes room and its tags are cleared. - The owned answer is honoured only from the host that answered and was sent the credential, so a redirect to another host cannot vouch, and it never restores trust to a token that was already untrusted. A link the account vouches for runs trusted as served and is not kept as a copy. - The refresh-on-open gate read the wrong result of GetZapLinkHost and passed hosts recorded as not serving ZapScript. --- docs/api/methods.md | 2 +- pkg/database/database.go | 3 +- pkg/database/decks.go | 30 ++- pkg/database/userdb/decks.go | 115 +++++++++- pkg/database/userdb/decks_test.go | 92 ++++++++ .../20260919120000_deck_playlist_id.sql | 17 ++ pkg/database/userdb/sql.go | 2 +- pkg/database/userdb/userdb.go | 2 +- pkg/platforms/platforms.go | 6 +- pkg/service/decks/playlist.go | 140 +++++------- pkg/service/decks/playlist_fuzz_test.go | 97 +++------ pkg/service/decks/playlist_test.go | 182 ++++++++++------ .../FuzzParseDeckPlaylist/2d5fb789b19201ed | 3 - pkg/service/playlist_refresh.go | 8 +- pkg/service/playlist_refresh_test.go | 36 ++- pkg/service/playlists/playlists.go | 15 +- pkg/service/queues.go | 2 +- pkg/service/queues_playlist_test.go | 6 + pkg/testing/helpers/db_mocks.go | 11 + pkg/zapscript/commands.go | 21 +- pkg/zapscript/commands_test.go | 75 +++++++ pkg/zapscript/playlist_deck.go | 36 +-- pkg/zapscript/playlist_deck_test.go | 205 +++++++++++++----- pkg/zapscript/zaplinks.go | 13 +- pkg/zapscript/zaplinks_test.go | 48 ++++ 25 files changed, 829 insertions(+), 338 deletions(-) create mode 100644 pkg/database/userdb/migrations/20260919120000_deck_playlist_id.sql delete mode 100644 pkg/service/decks/testdata/fuzz/FuzzParseDeckPlaylist/2d5fb789b19201ed diff --git a/docs/api/methods.md b/docs/api/methods.md index fc9802eb1..107bfb874 100644 --- a/docs/api/methods.md +++ b/docs/api/methods.md @@ -4156,7 +4156,7 @@ A deck is a persistent, ordered list of games and cards the user keeps on the de Every deck has a twelve-character ID drawn from the Crockford base32 alphabet (digits and letters without I, L, O and U). Core stores and shows it lower-case and matches it without regard to case; IDs of eight characters from older decks are also accepted, and may use any digit or letter. The same ID names the deck on the device, in its `user:deck:` tag, on a card and on the account. -A deck opens as a playlist from ZapScript with `**playlist.open:deck://` (and `playlist.load` and `playlist.play` accept the same argument), from the API with `decks.open`, or by tapping the deck's ZapLink. A deck reached through its ZapLink is cached on the device as a read-only copy, so it opens offline next time; a deck the device already owns opens its own copy instead. The playlist ID is `ZON-` whichever way the deck was opened, so reopening the deck that is playing keeps its position. +A deck opens as a playlist from ZapScript with `**playlist.open:deck://` (and `playlist.load` and `playlist.play` accept the same argument), from the API with `decks.open`, or by tapping a ZapLink that serves it. Any ZapLink, on any host, whose ZapScript is a single `playlist.open`, `playlist.play` or `playlist.load` command carrying a JSON playlist is kept on the device as a read-only deck, so it opens offline next time, shows in the deck list and has its games tagged. The copy is known by the link it was fetched from and gets a deck ID minted on this device; nothing in the link or the playlist is used as its ID, so it can never replace or stand in for a deck the user owns. At most 200 decks are kept from links; past that the one fetched longest ago is removed. A deck made on the device opens as the playlist ID `deck://`, and a deck kept from a link opens as the `id` its playlist was served with, so reopening the deck that is playing keeps its position whichever way it was opened. A link the linked account vouches for as the user's own runs as served and is not kept as a copy. A deck item is either a `script` (a name and the ZapScript it runs) or a `card` (an online card by ID, with its scripts and display metadata as pulled). A game added from the local library is stored as a script item: Core composes `**launch.title:/` with the file's disambiguating tags, so the item names the same game on any device, and keeps the file it was added from as the item's `media` so this device launches exactly that file. Card and deck `metadata` are stored as received and returned verbatim. diff --git a/pkg/database/database.go b/pkg/database/database.go index 3c7dfca58..cab869dcc 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -1174,6 +1174,7 @@ type UserDBI interface { UpdateDeck(deckID string, edit func(deck *Deck) error) (*Deck, error) DeleteDeck(deckID string) (bool, error) UpsertRemoteDeck(deck *Deck) (bool, error) + UpsertFetchedDeck(deck *Deck, maxCopies int) (FetchedDeckResult, error) SetDeckItemAnchor(itemDBID int64, anchor *DeckItemAnchor) error CountOwnedDecks() (int, error) RenameDeck(oldID, newID string) error @@ -1185,7 +1186,7 @@ type UserDBI interface { DeleteDeckSync(deckIDs []string) error ClearDeckSync() error UpdateZapLinkHost(host string, zapscript int) error - GetZapLinkHost(host string) (bool, bool, error) + GetZapLinkHost(host string) (supported, found bool, err error) GetSupportedZapLinkHosts() ([]string, error) PruneExpiredZapLinkHosts(olderThan time.Duration) (int64, error) UpdateZapLinkCache(url string, zapscript string) error diff --git a/pkg/database/decks.go b/pkg/database/decks.go index 00da8382f..567a90ccd 100644 --- a/pkg/database/decks.go +++ b/pkg/database/decks.go @@ -115,14 +115,28 @@ type Deck struct { Name string Description string SourceURL string - Metadata json.RawMessage - Items []DeckItem - DBID int64 - CreatedAt int64 - UpdatedAt int64 - FetchedAt int64 - ItemCount int - Owned bool + // PlaylistID is the playlist ID the deck's source served it under, kept + // verbatim so the local copy and the served playlist are one playlist. + // Empty for a deck that was not fetched from a link. + PlaylistID string + Metadata json.RawMessage + Items []DeckItem + DBID int64 + CreatedAt int64 + UpdatedAt int64 + FetchedAt int64 + ItemCount int + Owned bool +} + +// FetchedDeckResult reports what keeping a deck fetched from a link did. +type FetchedDeckResult struct { + // DeckID is the ID the copy is kept under on this device. + DeckID string + // Evicted lists the fetched decks removed to make room. + Evicted []string + // Changed reports whether the stored deck changed. + Changed bool } // NewDeckID mints a deck ID from the operating system's random source: diff --git a/pkg/database/userdb/decks.go b/pkg/database/userdb/decks.go index f008656c4..279402397 100644 --- a/pkg/database/userdb/decks.go +++ b/pkg/database/userdb/decks.go @@ -35,7 +35,8 @@ import ( "github.com/rs/zerolog/log" ) -const deckColumns = `DBID, DeckID, Name, Description, Owned, Metadata, SourceURL, FetchedAt, CreatedAt, UpdatedAt` +const deckColumns = `DBID, DeckID, Name, Description, Owned, Metadata, SourceURL, FetchedAt, CreatedAt, UpdatedAt, + PlaylistID` const deckItemColumns = `DBID, DeckDBID, Position, Kind, Name, ZapScript, CardID, Scripts, Metadata, SystemID, Path, MediaName, Tags, CreatedAt, UpdatedAt` @@ -241,10 +242,10 @@ func (db *UserDB) UpsertRemoteDeck(deck *database.Deck) (bool, error) { adoptRemoteDeckItemRows(stored, deck.Items) if _, execErr := tx.ExecContext(ctx, ` update Decks set Name = ?, Description = ?, Owned = ?, Metadata = ?, SourceURL = ?, - FetchedAt = ?, UpdatedAt = ? + PlaylistID = ?, FetchedAt = ?, UpdatedAt = ? where DBID = ?;`, deck.Name, deck.Description, deck.Owned, string(deck.Metadata), deck.SourceURL, - now, now, existing.DBID); execErr != nil { + deck.PlaylistID, now, now, existing.DBID); execErr != nil { return false, fmt.Errorf("failed to update remote deck: %w", execErr) } } @@ -257,6 +258,102 @@ func (db *UserDB) UpsertRemoteDeck(deck *database.Deck) (bool, error) { return changed, err } +// UpsertFetchedDeck stores a deck fetched from a link as a read-only copy. +// The copy is known by deck.SourceURL alone: a link fetched before updates its +// stored copy, and any other link gets a deck ID minted on this device, so +// nothing the source serves can name, replace or hide another deck. At most +// maxCopies fetched decks are kept; past that the ones fetched longest ago +// are removed and reported, so their tags can be cleared. +func (db *UserDB) UpsertFetchedDeck(deck *database.Deck, maxCopies int) (database.FetchedDeckResult, error) { + var result database.FetchedDeckResult + if db.sql.Load() == nil { + return result, ErrNullSQL + } + if deck.SourceURL == "" { + return result, errors.New("fetched deck has no source URL") + } + var deckID string + err := db.sql.Load().QueryRowContext(db.ctx, + `select DeckID from Decks where Owned = 0 and SourceURL = ?;`, deck.SourceURL, + ).Scan(&deckID) + isNew := errors.Is(err, sql.ErrNoRows) + switch { + case isNew: + if deckID, err = database.NewDeckID(); err != nil { + return result, fmt.Errorf("mint fetched deck ID: %w", err) + } + case err != nil: + return result, fmt.Errorf("failed to look up fetched deck: %w", err) + } + deck.DeckID, deck.Owned = deckID, false + changed, err := db.UpsertRemoteDeck(deck) + if err != nil { + return result, err + } + result.DeckID, result.Changed = deckID, changed + if isNew { + if result.Evicted, err = db.evictFetchedDecks(maxCopies); err != nil { + return result, err + } + } + return result, nil +} + +// evictFetchedDecks removes the fetched decks beyond the keep most recently +// fetched and returns their IDs. +func (db *UserDB) evictFetchedDecks(keep int) ([]string, error) { + var evicted []string + err := db.deckTx(func(ctx context.Context, tx *sql.Tx, _ int64) (bool, error) { + dbids, deckIDs, err := sqlFetchedDecksBeyond(ctx, tx, keep) + if err != nil { + return false, err + } + for _, dbid := range dbids { + if _, err = tx.ExecContext(ctx, `delete from DeckItems where DeckDBID = ?;`, dbid); err != nil { + return false, fmt.Errorf("failed to delete fetched deck items: %w", err) + } + if _, err = tx.ExecContext(ctx, `delete from Decks where DBID = ?;`, dbid); err != nil { + return false, fmt.Errorf("failed to delete fetched deck: %w", err) + } + } + evicted = deckIDs + return len(dbids) > 0, nil + }) + if err != nil { + return nil, err + } + return evicted, nil +} + +func sqlFetchedDecksBeyond( + ctx context.Context, q deckQueryable, keep int, +) (dbids []int64, deckIDs []string, err error) { + rows, err := q.QueryContext(ctx, ` + select DBID, DeckID from Decks where Owned = 0 and SourceURL != '' + order by FetchedAt desc, DBID desc limit -1 offset ?;`, keep) + if err != nil { + return nil, nil, fmt.Errorf("failed to query fetched decks: %w", err) + } + defer func() { + if closeErr := rows.Close(); closeErr != nil { + log.Warn().Err(closeErr).Msg("failed to close sql rows") + } + }() + for rows.Next() { + var dbid int64 + var deckID string + if scanErr := rows.Scan(&dbid, &deckID); scanErr != nil { + return nil, nil, fmt.Errorf("failed to scan fetched deck: %w", scanErr) + } + dbids = append(dbids, dbid) + deckIDs = append(deckIDs, deckID) + } + if err = rows.Err(); err != nil { + return nil, nil, fmt.Errorf("failed to iterate fetched decks: %w", err) + } + return dbids, deckIDs, nil +} + // touchUnchangedRemoteDeck records that a deck was fetched when the arriving // copy holds nothing the stored one does not. Rewriting the unchanged rows // would move every item to a new row, drop the local file each item is linked @@ -291,6 +388,7 @@ func (db *UserDB) touchUnchangedRemoteDeck(deck *database.Deck) (bool, error) { func sameRemoteDeck(stored, arriving *database.Deck) bool { if stored.Name != arriving.Name || stored.Description != arriving.Description || stored.SourceURL != arriving.SourceURL || stored.Owned != arriving.Owned || + stored.PlaylistID != arriving.PlaylistID || !bytes.Equal(stored.Metadata, arriving.Metadata) || len(stored.Items) != len(arriving.Items) { return false @@ -435,11 +533,12 @@ func sqlCountOwnedDecks(ctx context.Context, q deckQueryable) (int, error) { func sqlInsertDeck(ctx context.Context, q deckQueryable, deck *database.Deck) error { err := q.QueryRowContext(ctx, ` - insert into Decks(DeckID, Name, Description, Owned, Metadata, SourceURL, FetchedAt, CreatedAt, UpdatedAt) - values (?, ?, ?, ?, ?, ?, ?, ?, ?) + insert into Decks(DeckID, Name, Description, Owned, Metadata, SourceURL, FetchedAt, CreatedAt, UpdatedAt, + PlaylistID) + values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) returning DBID;`, deck.DeckID, deck.Name, deck.Description, deck.Owned, string(deck.Metadata), deck.SourceURL, - deck.FetchedAt, deck.CreatedAt, deck.UpdatedAt, + deck.FetchedAt, deck.CreatedAt, deck.UpdatedAt, deck.PlaylistID, ).Scan(&deck.DBID) if err != nil { return fmt.Errorf("failed to insert deck: %w", err) @@ -552,7 +651,7 @@ func scanDeck(scan func(dest ...any) error) (*database.Deck, error) { var metadata string if err := scan( &deck.DBID, &deck.DeckID, &deck.Name, &deck.Description, &deck.Owned, &metadata, - &deck.SourceURL, &deck.FetchedAt, &deck.CreatedAt, &deck.UpdatedAt, + &deck.SourceURL, &deck.FetchedAt, &deck.CreatedAt, &deck.UpdatedAt, &deck.PlaylistID, ); err != nil { return nil, err } @@ -604,7 +703,7 @@ func sqlListDecks(ctx context.Context, q deckQueryable) ([]database.Deck, error) var metadata string if scanErr := rows.Scan( &deck.DBID, &deck.DeckID, &deck.Name, &deck.Description, &deck.Owned, &metadata, - &deck.SourceURL, &deck.FetchedAt, &deck.CreatedAt, &deck.UpdatedAt, &deck.ItemCount, + &deck.SourceURL, &deck.FetchedAt, &deck.CreatedAt, &deck.UpdatedAt, &deck.PlaylistID, &deck.ItemCount, ); scanErr != nil { return nil, fmt.Errorf("failed to scan deck: %w", scanErr) } diff --git a/pkg/database/userdb/decks_test.go b/pkg/database/userdb/decks_test.go index ded27a0cf..63283da55 100644 --- a/pkg/database/userdb/decks_test.go +++ b/pkg/database/userdb/decks_test.go @@ -508,3 +508,95 @@ func TestUpdateDeck_UnlockedDeckStillEdits(t *testing.T) { require.NoError(t, err) assert.True(t, existed) } + +func TestUpsertFetchedDeck(t *testing.T) { + t.Parallel() + db, cleanup := setupTempUserDB(t) + t.Cleanup(cleanup) + require.NoError(t, db.CreateDeck(&database.Deck{DeckID: "aaaaaaaaaaaa", Name: "Mine", Owned: true})) + + fetched := func(source, playlistID string) *database.Deck { + return &database.Deck{ + // Whatever a caller or a source puts here is never used as the key. + DeckID: "aaaaaaaaaaaa", Owned: true, + Name: "Theirs", SourceURL: source, PlaylistID: playlistID, + Items: []database.DeckItem{scriptItem("A", "**a")}, + } + } + const link = "https://decks.example/shelf/1?ref=a" + + first, err := db.UpsertFetchedDeck(fetched(link, "party-list"), 10) + require.NoError(t, err) + assert.True(t, first.Changed) + assert.Empty(t, first.Evicted) + assert.True(t, database.IsMintedDeckID(first.DeckID)) + assert.NotEqual(t, "aaaaaaaaaaaa", first.DeckID, "the ID is minted here, never taken from the fetch") + got, err := db.GetDeck(first.DeckID) + require.NoError(t, err) + assert.False(t, got.Owned, "a fetched deck is never an owned one") + assert.Equal(t, link, got.SourceURL) + assert.Equal(t, "party-list", got.PlaylistID) + mine, err := db.GetDeck("aaaaaaaaaaaa") + require.NoError(t, err) + assert.Equal(t, "Mine", mine.Name) + assert.True(t, mine.Owned) + + // The same link is the same deck; a fetch that brings nothing new is not a change. + again, err := db.UpsertFetchedDeck(fetched(link, "party-list"), 10) + require.NoError(t, err) + assert.Equal(t, first.DeckID, again.DeckID) + assert.False(t, again.Changed) + + // A playlist ID the source changed follows the fetch. + again, err = db.UpsertFetchedDeck(fetched(link, "renamed"), 10) + require.NoError(t, err) + assert.Equal(t, first.DeckID, again.DeckID) + assert.True(t, again.Changed) + got, err = db.GetDeck(first.DeckID) + require.NoError(t, err) + assert.Equal(t, "renamed", got.PlaylistID) + + // Any other link, however alike, is another deck. + other, err := db.UpsertFetchedDeck(fetched(link+"b", "party-list"), 10) + require.NoError(t, err) + assert.NotEqual(t, first.DeckID, other.DeckID) + + _, err = db.UpsertFetchedDeck(fetched("", "p"), 10) + require.Error(t, err, "a fetched deck is known by its source") +} + +func TestUpsertFetchedDeckEvictsTheLongestUnfetched(t *testing.T) { + t.Parallel() + db, cleanup := setupTempUserDB(t) + t.Cleanup(cleanup) + require.NoError(t, db.CreateDeck(&database.Deck{DeckID: "aaaaaaaaaaaa", Name: "Mine", Owned: true})) + + ids := make([]string, 0, 3) + for i := range 3 { + result, err := db.UpsertFetchedDeck(&database.Deck{ + Name: "Theirs", SourceURL: fmt.Sprintf("https://decks.example/%d", i), + Items: []database.DeckItem{scriptItem("A", "**a")}, + }, 3) + require.NoError(t, err) + require.Empty(t, result.Evicted) + ids = append(ids, result.DeckID) + // Fetch times are whole seconds; order the copies explicitly. + _, err = db.sql.Load().ExecContext(t.Context(), + `update Decks set FetchedAt = ? where DeckID = ?;`, 100+i, result.DeckID) + require.NoError(t, err) + } + + result, err := db.UpsertFetchedDeck(&database.Deck{ + Name: "Theirs", SourceURL: "https://decks.example/new", + Items: []database.DeckItem{scriptItem("A", "**a")}, + }, 3) + require.NoError(t, err) + assert.Equal(t, []string{ids[0]}, result.Evicted, "the copy fetched longest ago makes room") + _, err = db.GetDeck(ids[0]) + require.ErrorIs(t, err, database.ErrDeckNotFound) + _, err = db.GetDeck("aaaaaaaaaaaa") + require.NoError(t, err, "an owned deck is never evicted") + all, err := db.ListDecks() + require.NoError(t, err) + assert.Len(t, all, 4) +} diff --git a/pkg/database/userdb/migrations/20260919120000_deck_playlist_id.sql b/pkg/database/userdb/migrations/20260919120000_deck_playlist_id.sql new file mode 100644 index 000000000..a0a4253a9 --- /dev/null +++ b/pkg/database/userdb/migrations/20260919120000_deck_playlist_id.sql @@ -0,0 +1,17 @@ +-- +goose Up +-- +goose StatementBegin + +-- A deck kept from a ZapLink is known by the link it came from, and opens as +-- the playlist ID its source served, so the copy and the served playlist are +-- the same playlist. A deck made on this device leaves PlaylistID empty. +alter table Decks add column PlaylistID text not null default ''; + +create unique index decks_fetched_source_idx on Decks (SourceURL) where Owned = 0 and SourceURL != ''; + +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +drop index decks_fetched_source_idx; +alter table Decks drop column PlaylistID; +-- +goose StatementEnd diff --git a/pkg/database/userdb/sql.go b/pkg/database/userdb/sql.go index 6bae137db..00097a54e 100644 --- a/pkg/database/userdb/sql.go +++ b/pkg/database/userdb/sql.go @@ -478,7 +478,7 @@ func sqlUpdateZapLinkHost(ctx context.Context, db *sql.DB, host string, zapscrip return nil } -func sqlGetZapLinkHost(ctx context.Context, db *sql.DB, host string) (found, zapScript bool, err error) { +func sqlGetZapLinkHost(ctx context.Context, db *sql.DB, host string) (supported, found bool, err error) { row := db.QueryRowContext(ctx, ` SELECT ZapScript FROM ZapLinkHosts WHERE Host = ?; `, host) diff --git a/pkg/database/userdb/userdb.go b/pkg/database/userdb/userdb.go index 456108573..f41083b8d 100644 --- a/pkg/database/userdb/userdb.go +++ b/pkg/database/userdb/userdb.go @@ -281,7 +281,7 @@ func (db *UserDB) UpdateZapLinkHost(host string, zapscript int) error { return sqlUpdateZapLinkHost(db.ctx, db.sql.Load(), host, zapscript) } -func (db *UserDB) GetZapLinkHost(host string) (found, zapScript bool, err error) { +func (db *UserDB) GetZapLinkHost(host string) (supported, found bool, err error) { return sqlGetZapLinkHost(db.ctx, db.sql.Load(), host) } diff --git a/pkg/platforms/platforms.go b/pkg/platforms/platforms.go index 58abf2dc8..65131e71f 100644 --- a/pkg/platforms/platforms.go +++ b/pkg/platforms/platforms.go @@ -222,11 +222,7 @@ type CmdEnv struct { // in the background as the deck opens. It returns at once and never // blocks the open; nil when no sync is configured. RefreshOwnedDeck func(ctx context.Context, deckID string) - // TrustedDeckID names a deck whose link the linked account vouched for - // as the user's own for this command, so a cached copy of it opens - // trusted this once. Empty otherwise. - TrustedDeckID string - PlaybackManager audio.PlaybackManager + PlaybackManager audio.PlaybackManager // LauncherCache resolves the launcher behind the active media. It holds // launchers the platform cannot build itself, so it is the only complete // source; never resolve a launcher ID from Platform.Launchers alone. diff --git a/pkg/service/decks/playlist.go b/pkg/service/decks/playlist.go index 4e605682c..4a286d824 100644 --- a/pkg/service/decks/playlist.go +++ b/pkg/service/decks/playlist.go @@ -24,11 +24,9 @@ import ( "encoding/json" "errors" "fmt" - "net/url" "strings" "github.com/ZaparooProject/go-zapscript" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" "github.com/rs/zerolog/log" ) @@ -37,11 +35,11 @@ const ( // URIScheme names a local deck in a playlist command: // **playlist.open:deck://<id>. URIScheme = "deck" - // PlaylistIDPrefix leads the playlist ID a deck opens as. It is the - // prefix the online service uses when it serves a deck as a playlist, so - // a deck opened locally and one opened from its ZapLink are the same - // playlist. - PlaylistIDPrefix = "ZON-" + // MaxFetchedDecks bounds how many decks kept from links a device holds. + // Any link that serves a playlist is kept, so the bound is what stops + // taps piling decks up without end; the ones fetched longest ago go + // first. + MaxFetchedDecks = 200 ) // ErrNoUserDB reports a deck store reached with no user database open. @@ -71,42 +69,17 @@ func ParseDeckURI(arg string) (string, bool) { return deckID, true } -// PlaylistID returns the ID a deck opens as. -func PlaylistID(deckID string) string { - return PlaylistIDPrefix + strings.ToLower(deckID) -} - -// DeckIDFromZapLinkURL returns the deck a ZapLink URL names. Only the -// official hosts serve decks, under a path of "d" followed by the deck ID; -// any other URL, including a card link, is not a deck. -func DeckIDFromZapLinkURL(raw string) (string, bool) { - u, err := url.Parse(raw) - if err != nil || !strings.EqualFold(u.Scheme, "https") { - return "", false - } - official := false - for _, host := range config.OfficialAuthHosts { - if strings.EqualFold(u.Hostname(), host) { - official = true - break - } - } - if !official { - return "", false - } - path := strings.TrimPrefix(u.Path, "/") - if len(path) < 2 || (path[0] != 'd' && path[0] != 'D') || strings.Contains(path, "/") { - return "", false - } - deckID, err := database.NormalizeDeckID(path[1:]) - if err != nil { - return "", false +// PlaylistID returns the ID a deck opens as. A deck kept from a link opens as +// the playlist ID its source served, so the local copy and the served +// playlist are one playlist; any other deck opens as its own deck:// URI. +func PlaylistID(deck *database.Deck) string { + if deck.PlaylistID != "" { + return deck.PlaylistID } - return deckID, true + return DeckURI(strings.ToLower(deck.DeckID)) } -// PlaylistArg is the JSON argument of a playlist command, the shape the -// online service serves a deck in. +// PlaylistArg is the JSON argument of a playlist command. type PlaylistArg struct { ID string `json:"id"` Name string `json:"name"` @@ -119,44 +92,47 @@ type PlaylistArgItem struct { ZapScript string `json:"zapscript"` } -// ParseDeckPlaylist reads a fetched ZapLink body as the playlist a deck is -// served as: one playlist.open command whose JSON argument carries the -// deck's own playlist ID. Anything else is not that deck. -func ParseDeckPlaylist(body, deckID string) (PlaylistArg, bool) { - // The ID is checked here rather than trusted from the caller: this is what - // decides a fetched body is a given deck, so a body must never be able to - // match an ID that is not one a deck could have. - normalized, err := database.NormalizeDeckID(deckID) - if err != nil { - return PlaylistArg{}, false - } +// ParseServedPlaylist reads a fetched ZapLink body as a playlist to keep: one +// playlist.open, playlist.play or playlist.load command whose only argument +// is a JSON playlist with at least one item to run. It returns the command as +// served and its playlist. What the link looks like and what the playlist +// calls itself play no part, so every host's playlist is kept alike. +func ParseServedPlaylist(body string) (zapscript.Command, PlaylistArg, bool) { parsed, err := zapscript.NewParser(body).ParseScript() if err != nil || len(parsed.Cmds) != 1 { - return PlaylistArg{}, false + return zapscript.Command{}, PlaylistArg{}, false } cmd := parsed.Cmds[0] - if cmd.Name != zapscript.ZapScriptCmdPlaylistOpen || len(cmd.Args) != 1 { - return PlaylistArg{}, false + switch cmd.Name { + case zapscript.ZapScriptCmdPlaylistOpen, zapscript.ZapScriptCmdPlaylistPlay, zapscript.ZapScriptCmdPlaylistLoad: + default: + return zapscript.Command{}, PlaylistArg{}, false + } + if len(cmd.Args) != 1 { + return zapscript.Command{}, PlaylistArg{}, false } var arg PlaylistArg if jsonErr := json.Unmarshal([]byte(cmd.Args[0]), &arg); jsonErr != nil { - return PlaylistArg{}, false + return zapscript.Command{}, PlaylistArg{}, false } - if !strings.EqualFold(arg.ID, PlaylistID(normalized)) { - return PlaylistArg{}, false + for _, item := range arg.Items { + if strings.TrimSpace(item.ZapScript) != "" { + return cmd, arg, true + } } - return arg, true + return zapscript.Command{}, PlaylistArg{}, false } -// StoreFetchedDeck caches a deck served through a ZapLink as a read-only -// copy. Every served entry becomes a script item: a card with several -// scripts is served as a nested playlist command and is kept as that script. -// A deck this device owns is left alone, since the owned copy is the one to -// open. A deck that changed has its membership tags queued, so the tags and -// the files its items link to follow the fetch in the background. -func StoreFetchedDeck(db *database.Database, sourceURL, deckID string, arg *PlaylistArg) error { +// StoreFetchedDeck keeps a playlist served through a ZapLink as a read-only +// deck and returns the deck's ID on this device. The copy is known by the +// link it came from and nothing else, so it never stands in for, or is hidden +// by, a deck the user owns. Every served entry becomes a script item: a card +// with several scripts is served as a nested playlist command and is kept as +// that script. A deck that changed has its membership tags queued, so the +// tags and the files its items link to follow the fetch in the background. +func StoreFetchedDeck(db *database.Database, sourceURL string, arg *PlaylistArg) (string, error) { if db == nil || db.UserDB == nil { - return ErrNoUserDB + return "", ErrNoUserDB } items := make([]database.DeckItem, 0, len(arg.Items)) for _, item := range arg.Items { @@ -168,25 +144,25 @@ func StoreFetchedDeck(db *database.Database, sourceURL, deckID string, arg *Play }) } if len(items) > database.DeckMaxItems { - return fmt.Errorf("%w: fetched deck %s has %d items", database.ErrDeckItemLimit, deckID, len(items)) + return "", fmt.Errorf("%w: fetched deck has %d items", database.ErrDeckItemLimit, len(items)) } name := strings.TrimSpace(arg.Name) if name == "" { - name = strings.ToUpper(deckID) - } - changed, err := db.UserDB.UpsertRemoteDeck(&database.Deck{ - DeckID: deckID, Name: name, Owned: false, SourceURL: sourceURL, Items: items, - }) - if errors.Is(err, database.ErrDeckOwned) { - return nil + name = sourceURL } + result, err := db.UserDB.UpsertFetchedDeck(&database.Deck{ + Name: name, SourceURL: sourceURL, PlaylistID: arg.ID, Items: items, + }, MaxFetchedDecks) if err != nil { - return fmt.Errorf("store fetched deck %s: %w", deckID, err) + return "", fmt.Errorf("store fetched deck: %w", err) + } + if result.Changed { + db.QueueDeckTags(result.DeckID) } - if changed { - db.QueueDeckTags(deckID) + if len(result.Evicted) > 0 { + db.QueueDeckTags(result.Evicted...) } - return nil + return result.DeckID, nil } // PlaylistItems turns a deck into the entries a playlist runs. A game item @@ -214,7 +190,7 @@ func PlaylistItems(ctx context.Context, mediaDB database.MediaDBI, deck *databas } out = append(out, PlaylistItem{Name: item.Name, ZapScript: script}) case database.DeckItemKindCard: - if script, ok := cardPlaylistScript(item); ok { + if script, ok := cardPlaylistScript(deck, item); ok { out = append(out, PlaylistItem{Name: item.Name, ZapScript: script}) } } @@ -222,7 +198,7 @@ func PlaylistItems(ctx context.Context, mediaDB database.MediaDBI, deck *databas return out } -func cardPlaylistScript(item *database.DeckItem) (string, bool) { +func cardPlaylistScript(deck *database.Deck, item *database.DeckItem) (string, bool) { switch len(item.Scripts) { case 0: return "", false @@ -230,7 +206,9 @@ func cardPlaylistScript(item *database.DeckItem) (string, bool) { return item.Scripts[0].ZapScript, item.Scripts[0].ZapScript != "" } nested := PlaylistArg{ - ID: PlaylistID(item.CardID), Name: item.Name, Items: make([]PlaylistArgItem, 0, len(item.Scripts)), + ID: DeckURI(strings.ToLower(deck.DeckID)) + "/" + item.CardID, + Name: item.Name, + Items: make([]PlaylistArgItem, 0, len(item.Scripts)), } for _, s := range item.Scripts { nested.Items = append(nested.Items, PlaylistArgItem{Name: s.Name, ZapScript: s.ZapScript}) diff --git a/pkg/service/decks/playlist_fuzz_test.go b/pkg/service/decks/playlist_fuzz_test.go index 1f61f6363..32695ab09 100644 --- a/pkg/service/decks/playlist_fuzz_test.go +++ b/pkg/service/decks/playlist_fuzz_test.go @@ -20,86 +20,51 @@ package decks_test import ( - "net/url" "strings" "testing" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" + "github.com/ZaparooProject/go-zapscript" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/decks" ) -// FuzzParseDeckPlaylist feeds arbitrary bodies to the parser that reads a -// fetched ZapLink as a served deck. The body comes off the network, so the -// parser must answer for any bytes at all rather than panicking, and must -// never claim a body is a deck whose ID it does not carry. -func FuzzParseDeckPlaylist(f *testing.F) { - f.Add(`**playlist.open:{"id":"ZON-0123456789ab","name":"D","items":[]}`, "0123456789ab") - f.Add(`**playlist.open:{"id":"ZON-0123456789AB","name":"D",`+ - `"items":[{"name":"A","zapscript":"**launch.system:NES"}]}`, "0123456789ab") - f.Add(`**playlist.open:{"id":"ZON-zzzzzzzzzzzz"}`, "0123456789ab") - f.Add(`**playlist.open:/roms/list.pls`, "0123456789ab") - f.Add(`**launch.system:SNES`, "0123456789ab") - f.Add(`**playlist.open:{"id":`, "0123456789ab") - f.Add("", "") - f.Add(`**playlist.open:{"id":"ZON-0123456789ab"}||**stop`, "0123456789ab") - - f.Fuzz(func(t *testing.T, body, deckID string) { - arg, ok := decks.ParseDeckPlaylist(body, deckID) - if !ok { - return - } - // A body only parses as this deck when the ID is a real one and the - // served playlist names it, so a caller can trust what it stores. - normalized, err := database.NormalizeDeckID(deckID) - if err != nil { - t.Fatalf("accepted playlist for unusable deck id %q", deckID) - } - if want := decks.PlaylistID(normalized); !strings.EqualFold(arg.ID, want) { - t.Fatalf("accepted playlist %q for deck %q", arg.ID, want) - } - }) -} - -// FuzzDeckIDFromZapLinkURL feeds arbitrary URLs to the check that decides -// whether a tapped link is a deck this device may cache. Anything it accepts -// must be an official host and a usable deck ID. -func FuzzDeckIDFromZapLinkURL(f *testing.F) { - f.Add("https://zpr.au/d0123456789ab") - f.Add("https://zpr.au/DABCDEFGH") - f.Add("https://edge.zaparoo.com/d0123456789ab") - f.Add("https://zpr.au/c0123456789ab") - f.Add("http://zpr.au/d0123456789ab") - f.Add("https://example.com/d0123456789ab") - f.Add("https://zpr.au/d0123456789ab/extra") - f.Add("https://zpr.au@evil.test/d0123456789ab") - f.Add("not a url %%") +// FuzzParseServedPlaylist feeds arbitrary bodies to the parser that decides +// whether a fetched ZapLink serves a playlist to keep as a deck. The body +// comes off the network, so the parser must answer for any bytes at all +// rather than panicking, and must only ever accept a lone playlist command +// that has something to run. +func FuzzParseServedPlaylist(f *testing.F) { + f.Add(`**playlist.open:{"id":"ZON-LHM6N9T8","name":"D","items":[{"name":"A","zapscript":"@Genesis/A"}]}`) + f.Add(`**playlist.play:{"id":"party-list","items":[{"zapscript":"**launch.system:NES"}]}?mode=shuffle`) + f.Add(`**playlist.load:{"items":[{"zapscript":"**stop"}]}`) + f.Add(`**playlist.open:{"id":"p","name":"D","items":[]}`) + f.Add(`**playlist.open:{"id":"p","items":[{"zapscript":" "}]}`) + f.Add(`**playlist.open:/roms/list.pls`) + f.Add(`**playlist.open:deck://0123456789ab`) + f.Add(`**launch.system:SNES`) + f.Add(`**playlist.open:{"id":`) f.Add("") + f.Add(`**playlist.open:{"id":"p","items":[{"zapscript":"**stop"}]}||**stop`) - f.Fuzz(func(t *testing.T, raw string) { - deckID, ok := decks.DeckIDFromZapLinkURL(raw) + f.Fuzz(func(t *testing.T, body string) { + cmd, arg, ok := decks.ParseServedPlaylist(body) if !ok { return } - if _, err := database.NormalizeDeckID(deckID); err != nil { - t.Fatalf("accepted %q as deck id %q", raw, deckID) - } - u, err := url.Parse(raw) - if err != nil { - t.Fatalf("accepted unparseable url %q", raw) + switch cmd.Name { + case zapscript.ZapScriptCmdPlaylistOpen, zapscript.ZapScriptCmdPlaylistPlay, + zapscript.ZapScriptCmdPlaylistLoad: + default: + t.Fatalf("accepted command %q", cmd.Name) } - if !strings.EqualFold(u.Scheme, "https") { - t.Fatalf("accepted non-https url %q", raw) + if len(cmd.Args) != 1 { + t.Fatalf("accepted a playlist command with %d arguments", len(cmd.Args)) } - official := false - for _, host := range config.OfficialAuthHosts { - if strings.EqualFold(u.Hostname(), host) { - official = true - break - } + runnable := false + for _, item := range arg.Items { + runnable = runnable || strings.TrimSpace(item.ZapScript) != "" } - if !official { - t.Fatalf("accepted unofficial host in %q", raw) + if !runnable { + t.Fatalf("accepted a playlist with nothing to run: %q", body) } }) } diff --git a/pkg/service/decks/playlist_test.go b/pkg/service/decks/playlist_test.go index 15f1ea1c2..8f29f141b 100644 --- a/pkg/service/decks/playlist_test.go +++ b/pkg/service/decks/playlist_test.go @@ -34,7 +34,6 @@ import ( func TestDeckURI(t *testing.T) { t.Parallel() assert.Equal(t, "deck://0123456789ab", decks.DeckURI("0123456789ab")) - assert.Equal(t, "ZON-0123456789ab", decks.PlaylistID("0123456789AB")) for raw, want := range map[string]string{ "deck://0123456789ab": "0123456789ab", @@ -52,82 +51,134 @@ func TestDeckURI(t *testing.T) { } } -func TestDeckIDFromZapLinkURL(t *testing.T) { - t.Parallel() - for raw, want := range map[string]string{ - "https://zpr.au/d0123456789ab": "0123456789ab", - "https://zpr.au/DABCDEFGH": "abcdefgh", - "https://edge.zaparoo.com/d0123456789ab": "0123456789ab", - "https://zpr.au/c0123456789ab": "", - "https://zpr.au/0123456789ab": "", - "https://zpr.au/d0123456789ab/extra": "", - "http://zpr.au/d0123456789ab": "", - "https://example.com/d0123456789ab": "", - "https://zpr.au/dIIIIIIIIIII": "", - "not a url %%": "", - } { - got, ok := decks.DeckIDFromZapLinkURL(raw) - assert.Equal(t, want != "", ok, raw) - assert.Equal(t, want, got, raw) - } -} +// The links a deck is reached through. One is a real link on the hosted +// service and the other belongs to nobody in particular: nothing about a host +// or a link's shape may decide how its playlist is treated. +const ( + hostedDeckLink = "https://zpr.au/d$lhm6n9t8" + thirdPartyDeckLink = "https://decks.example/shelf/anything/at/all?ref=1" +) -func servedDeck(t *testing.T, id string, items ...decks.PlaylistArgItem) string { +func servedPlaylist(t *testing.T, cmdName, id string, items ...decks.PlaylistArgItem) string { t.Helper() - encoded, err := json.Marshal(decks.PlaylistArg{ID: "ZON-" + id, Name: "Served", Items: items}) + encoded, err := json.Marshal(decks.PlaylistArg{ID: id, Name: "Served", Items: items}) require.NoError(t, err) - return "**playlist.open:" + string(encoded) + return "**" + cmdName + ":" + string(encoded) } -func TestParseDeckPlaylist(t *testing.T) { +func TestParseServedPlaylist(t *testing.T) { t.Parallel() - body := servedDeck(t, "0123456789AB", decks.PlaylistArgItem{Name: "A", ZapScript: "**launch.system:SNES"}) - arg, ok := decks.ParseDeckPlaylist(body, "0123456789ab") - require.True(t, ok, "the served ID matches regardless of case") - assert.Equal(t, "Served", arg.Name) + item := decks.PlaylistArgItem{Name: "A", ZapScript: "**launch.system:SNES"} + + // The body the hosted service serves for a deck, as fetched. + hosted := `**playlist.open:{"id":"ZON-LHM6N9T8","name":"My Favourites","items":[` + + `{"name":"Gunstar Heroes","zapscript":"@Genesis/Gunstar Heroes (year:1993)"}]}` + cmd, arg, ok := decks.ParseServedPlaylist(hosted) + require.True(t, ok) + assert.Equal(t, zapscript.ZapScriptCmdPlaylistOpen, cmd.Name) + assert.Equal(t, "ZON-LHM6N9T8", arg.ID, "the served ID is kept as it is, whatever it looks like") + assert.Equal(t, "My Favourites", arg.Name) require.Len(t, arg.Items, 1) - _, ok = decks.ParseDeckPlaylist(body, "zzzzzzzzzzzz") - assert.False(t, ok, "a playlist for another ID is not this deck") - _, ok = decks.ParseDeckPlaylist("**launch.system:SNES", "0123456789ab") - assert.False(t, ok) - _, ok = decks.ParseDeckPlaylist(body+"||**stop", "0123456789ab") - assert.False(t, ok, "a body with more than the playlist is not a served deck") - _, ok = decks.ParseDeckPlaylist("**playlist.open:/roms/list.pls", "0123456789ab") - assert.False(t, ok) + for _, id := range []string{"party-list", "", "deck://0123456789ab"} { + _, arg, ok = decks.ParseServedPlaylist(servedPlaylist(t, zapscript.ZapScriptCmdPlaylistOpen, id, item)) + require.True(t, ok, "a playlist is kept whatever it calls itself: %q", id) + assert.Equal(t, id, arg.ID) + } + for _, name := range []string{zapscript.ZapScriptCmdPlaylistPlay, zapscript.ZapScriptCmdPlaylistLoad} { + cmd, _, ok = decks.ParseServedPlaylist(servedPlaylist(t, name, "p", item) + "?mode=shuffle") + require.True(t, ok, name) + assert.Equal(t, name, cmd.Name, "the command is returned as served") + assert.Equal(t, "shuffle", cmd.AdvArgs.Get(zapscript.KeyMode)) + } + + for name, body := range map[string]string{ + "not a playlist": "**launch.system:SNES", + "more than a playlist": servedPlaylist(t, zapscript.ZapScriptCmdPlaylistOpen, "p", item) + "||**stop", + "a playlist file": "**playlist.open:/roms/list.pls", + "a local deck": "**playlist.open:deck://0123456789ab", + "nothing to run": servedPlaylist(t, zapscript.ZapScriptCmdPlaylistOpen, "p", + decks.PlaylistArgItem{Name: "Empty", ZapScript: " "}), + "no items": servedPlaylist(t, zapscript.ZapScriptCmdPlaylistOpen, "p"), + "another command": "**playlist.goto:" + `{"id":"p","items":[{"zapscript":"**stop"}]}`, + "unparseable json": "**playlist.open:{not json", + } { + _, _, ok = decks.ParseServedPlaylist(body) + assert.False(t, ok, name) + } } +// Every host's playlist is kept the same way, and what it is kept under comes +// from this device alone. func TestStoreFetchedDeck(t *testing.T) { t.Parallel() - f := newProjectFixture(t) + for name, link := range map[string]string{"hosted": hostedDeckLink, "third party": thirdPartyDeckLink} { + t.Run(name, func(t *testing.T) { + t.Parallel() + f := newProjectFixture(t) + nested := servedPlaylist(t, zapscript.ZapScriptCmdPlaylistOpen, "card1234", + decks.PlaylistArgItem{ZapScript: "**a"}, decks.PlaylistArgItem{ZapScript: "**b"}) + arg := decks.PlaylistArg{ID: "party-list", Name: " Theirs ", Items: []decks.PlaylistArgItem{ + {Name: "Game", ZapScript: "**launch.title:SNES/Game"}, + {Name: "Card", ZapScript: nested}, + {Name: "Empty", ZapScript: " "}, + }} + deckID, err := decks.StoreFetchedDeck(f.db, link, &arg) + require.NoError(t, err) + assert.True(t, database.IsMintedDeckID(deckID), "the deck ID is minted here, not taken from the link") - nested := servedDeck(t, "card1234", - decks.PlaylistArgItem{ZapScript: "**a"}, decks.PlaylistArgItem{ZapScript: "**b"}) - arg := decks.PlaylistArg{ID: "ZON-0123456789AB", Name: " Theirs ", Items: []decks.PlaylistArgItem{ - {Name: "Game", ZapScript: "**launch.title:SNES/Game"}, - {Name: "Card", ZapScript: nested}, - {Name: "Empty", ZapScript: " "}, - }} - require.NoError(t, decks.StoreFetchedDeck(f.db, "https://zpr.au/d0123456789ab", "0123456789ab", &arg)) - stored, err := f.userDB.GetDeck("0123456789ab") - require.NoError(t, err) - assert.False(t, stored.Owned) - assert.Equal(t, "Theirs", stored.Name) - assert.Equal(t, "https://zpr.au/d0123456789ab", stored.SourceURL) - require.Len(t, stored.Items, 2, "entries with no script are dropped") - assert.Equal(t, nested, stored.Items[1].ZapScript, "a multi-script card is kept as its nested playlist") + stored, err := f.userDB.GetDeck(deckID) + require.NoError(t, err) + assert.False(t, stored.Owned) + assert.Equal(t, "Theirs", stored.Name) + assert.Equal(t, link, stored.SourceURL) + assert.Equal(t, "party-list", stored.PlaylistID) + assert.Equal(t, "party-list", decks.PlaylistID(stored), "the copy opens as the playlist it was served as") + require.Len(t, stored.Items, 2, "entries with no script are dropped") + assert.Equal(t, nested, stored.Items[1].ZapScript, "a multi-script card is kept as its nested playlist") + assert.Equal(t, []string{deckID}, f.tagQueue.queued(), + "a newly kept deck is queued so its tags and file links follow") - assert.Equal(t, []string{"0123456789ab"}, f.tagQueue.queued(), - "a newly cached deck is queued so its tags and file links follow") + again, err := decks.StoreFetchedDeck(f.db, link, &arg) + require.NoError(t, err) + assert.Equal(t, deckID, again, "the same link is the same deck") + }) + } +} - // An owned deck of the same ID is left alone without an error. +// A served playlist can never stand in for, or be hidden by, a deck the user +// owns, whatever ID it serves or link it sits at. +func TestStoreFetchedDeckNeverTouchesAnOwnedDeck(t *testing.T) { + t.Parallel() + f := newProjectFixture(t) require.NoError(t, f.userDB.CreateDeck(&database.Deck{DeckID: "aaaaaaaaaaaa", Name: "Mine", Owned: true})) - require.NoError(t, decks.StoreFetchedDeck(f.db, "https://zpr.au/daaaaaaaaaaaa", "aaaaaaaaaaaa", &arg)) mine, err := f.userDB.GetDeck("aaaaaaaaaaaa") require.NoError(t, err) + + arg := decks.PlaylistArg{ID: decks.PlaylistID(mine), Name: "Theirs", Items: []decks.PlaylistArgItem{ + {Name: "Game", ZapScript: "**launch.system:SNES"}, + }} + deckID, err := decks.StoreFetchedDeck(f.db, "https://decks.example/daaaaaaaaaaaa", &arg) + require.NoError(t, err) + assert.NotEqual(t, "aaaaaaaaaaaa", deckID) + + mine, err = f.userDB.GetDeck("aaaaaaaaaaaa") + require.NoError(t, err) assert.Equal(t, "Mine", mine.Name) assert.True(t, mine.Owned) - assert.Equal(t, []string{"0123456789ab"}, f.tagQueue.queued(), "an owned deck is not re-tagged by a fetch") + assert.Empty(t, mine.Items) + theirs, err := f.userDB.GetDeck(deckID) + require.NoError(t, err) + assert.Equal(t, "Theirs", theirs.Name) + assert.False(t, theirs.Owned) +} + +func TestPlaylistID(t *testing.T) { + t.Parallel() + assert.Equal(t, "deck://0123456789ab", decks.PlaylistID(&database.Deck{DeckID: "0123456789AB"}), + "a deck made on this device opens as its own URI") + kept := &database.Deck{DeckID: "0123456789ab", PlaylistID: "ZON-LHM6N9T8"} + assert.Equal(t, "ZON-LHM6N9T8", decks.PlaylistID(kept), "a deck kept from a link opens as it was served") } // A deck that is fetched again keeps the rows of the items it still holds, so @@ -138,14 +189,14 @@ func TestStoreFetchedDeckKeepsLocalLinksAcrossRefresh(t *testing.T) { t.Parallel() present := filepath.ToSlash(filepath.Join("roms", "NES", "Metroid (USA).nes")) f := newProjectFixture(t, present) - const deckID = "0123456789ab" - const src = "https://zpr.au/d0123456789ab" + const src = hostedDeckLink arg := decks.PlaylistArg{ID: "ZON-0123456789AB", Name: "Theirs", Items: []decks.PlaylistArgItem{ {Name: "Metroid", ZapScript: decks.TitleLaunchScript("NES", "Metroid", nil)}, {Name: "Other", ZapScript: "**launch.system:NES"}, }} - require.NoError(t, decks.StoreFetchedDeck(f.db, src, deckID, &arg)) + deckID, err := decks.StoreFetchedDeck(f.db, src, &arg) + require.NoError(t, err) // The tagger links the item to the file this device matched. relinked, err := decks.ProjectDeck(f.ctx, f.deps, deckID) @@ -161,7 +212,8 @@ func TestStoreFetchedDeckKeepsLocalLinksAcrossRefresh(t *testing.T) { // The same deck served again changes nothing but the fetch time. before := preferencesRevision(t, f) - require.NoError(t, decks.StoreFetchedDeck(f.db, src, deckID, &arg)) + _, err = decks.StoreFetchedDeck(f.db, src, &arg) + require.NoError(t, err) same, err := f.userDB.GetDeck(deckID) require.NoError(t, err) assert.Equal(t, itemID, same.Items[0].DBID, "an unchanged item keeps its row") @@ -177,7 +229,8 @@ func TestStoreFetchedDeckKeepsLocalLinksAcrossRefresh(t *testing.T) { {Name: "New", ZapScript: "**launch.system:SNES"}, {Name: "Metroid", ZapScript: decks.TitleLaunchScript("NES", "Metroid", nil)}, } - require.NoError(t, decks.StoreFetchedDeck(f.db, src, deckID, &arg)) + _, err = decks.StoreFetchedDeck(f.db, src, &arg) + require.NoError(t, err) moved, err := f.userDB.GetDeck(deckID) require.NoError(t, err) require.Len(t, moved.Items, 3) @@ -187,7 +240,8 @@ func TestStoreFetchedDeckKeepsLocalLinksAcrossRefresh(t *testing.T) { // An item the source dropped takes its row and link with it. arg.Items = []decks.PlaylistArgItem{{Name: "Other", ZapScript: "**launch.system:NES"}} - require.NoError(t, decks.StoreFetchedDeck(f.db, src, deckID, &arg)) + _, err = decks.StoreFetchedDeck(f.db, src, &arg) + require.NoError(t, err) shrunk, err := f.userDB.GetDeck(deckID) require.NoError(t, err) require.Len(t, shrunk.Items, 1) @@ -238,7 +292,7 @@ func TestPlaylistItems(t *testing.T) { assert.Equal(t, zapscript.ZapScriptCmdPlaylistOpen, nested.Cmds[0].Name) var arg decks.PlaylistArg require.NoError(t, json.Unmarshal([]byte(nested.Cmds[0].Args[0]), &arg)) - assert.Equal(t, "ZON-card0002", arg.ID) + assert.Equal(t, "deck://0123456789ab/card0002", arg.ID) require.Len(t, arg.Items, 2) assert.Equal(t, "**b", arg.Items[1].ZapScript) } diff --git a/pkg/service/decks/testdata/fuzz/FuzzParseDeckPlaylist/2d5fb789b19201ed b/pkg/service/decks/testdata/fuzz/FuzzParseDeckPlaylist/2d5fb789b19201ed deleted file mode 100644 index 75ce7a2ec..000000000 --- a/pkg/service/decks/testdata/fuzz/FuzzParseDeckPlaylist/2d5fb789b19201ed +++ /dev/null @@ -1,3 +0,0 @@ -go test fuzz v1 -string("**plAYlist.open:{\"id\":\"ZON-\xd60\"}") -string("\xdb0") diff --git a/pkg/service/playlist_refresh.go b/pkg/service/playlist_refresh.go index 9ef09bfbf..7ab0b8164 100644 --- a/pkg/service/playlist_refresh.go +++ b/pkg/service/playlist_refresh.go @@ -65,9 +65,11 @@ func refreshOpenDeckPlaylist(ctx context.Context, svc *ServiceContext, notificat if err != nil { return } - playlistID := decks.PlaylistID(deckID) for _, active := range []*playlists.Playlist{svc.State.GetActivePlaylist(), svc.State.GetBackgroundPlaylist()} { - if active == nil || active.ID != playlistID { + // The deck the playlist was opened from decides the match, never the + // playlist's ID: any served playlist may call itself anything, + // including the ID a deck on this device opens as. + if active == nil || active.DeckID != deckID { continue } deck, getErr := svc.DB.UserDB.GetDeck(deckID) @@ -84,7 +86,7 @@ func refreshOpenDeckPlaylist(ctx context.Context, svc *ServiceContext, notificat // Setting it here would let an omission grant trust instead of // withholding it. refreshed := &playlists.Playlist{ - ID: playlistID, Name: deck.Name, Slot: active.Slot, Items: items, + ID: active.ID, DeckID: deckID, Name: deck.Name, Slot: active.Slot, Items: items, Loop: active.Loop, LoopOne: active.LoopOne, Refresh: true, } select { diff --git a/pkg/service/playlist_refresh_test.go b/pkg/service/playlist_refresh_test.go index 346d53638..bc9591595 100644 --- a/pkg/service/playlist_refresh_test.go +++ b/pkg/service/playlist_refresh_test.go @@ -53,8 +53,9 @@ func TestRefreshOpenDeckPlaylist_QueuesRefreshForTheOpenDeck(t *testing.T) { {Kind: database.DeckItemKindScript, Name: "Second", ZapScript: "**b"}, }, }, nil).Once() - active := playlists.NewPlaylist("ZON-0123456789ab", "Weekend", + active := playlists.NewPlaylist("deck://0123456789ab", "Weekend", []playlists.PlaylistItem{{Name: "First", ZapScript: "**a"}}) + active.DeckID = "0123456789ab" active.Index = 0 active.Playing = true svc.State.SetActivePlaylist(active) @@ -65,7 +66,7 @@ func TestRefreshOpenDeckPlaylist_QueuesRefreshForTheOpenDeck(t *testing.T) { select { case queued := <-svc.PlaylistQueue: assert.True(t, queued.Refresh) - assert.Equal(t, "ZON-0123456789ab", queued.ID) + assert.Equal(t, "deck://0123456789ab", queued.ID) assert.Equal(t, "Weekend Plus", queued.Name) assert.Len(t, queued.Items, 2) default: @@ -77,7 +78,8 @@ func TestRefreshOpenDeckPlaylist_QueuesRefreshForTheOpenDeck(t *testing.T) { func TestRefreshOpenDeckPlaylist_IgnoresOtherDecksAndActions(t *testing.T) { t.Parallel() svc := setupPlaylistTestEnv(t) - active := playlists.NewPlaylist("ZON-0123456789ab", "Weekend", []playlists.PlaylistItem{{ZapScript: "**a"}}) + active := playlists.NewPlaylist("deck://0123456789ab", "Weekend", []playlists.PlaylistItem{{ZapScript: "**a"}}) + active.DeckID = "0123456789ab" svc.State.SetActivePlaylist(active) other := decksChangedNotification(t, "bbbbbbbbbbbb", models.DecksChangedRefreshed) @@ -105,8 +107,9 @@ func TestWatchDecksForPlaylistRefresh_RefreshesOnNotification(t *testing.T) { DeckID: "0123456789ab", Name: "Weekend Plus", Owned: true, Items: []database.DeckItem{{Kind: database.DeckItemKindScript, Name: "First", ZapScript: "**a"}}, }, nil).Once() - active := playlists.NewPlaylist("ZON-0123456789ab", "Weekend", + active := playlists.NewPlaylist("deck://0123456789ab", "Weekend", []playlists.PlaylistItem{{Name: "First", ZapScript: "**a"}}) + active.DeckID = "0123456789ab" svc.State.SetActivePlaylist(active) ctx, cancel := context.WithCancel(context.Background()) @@ -133,7 +136,7 @@ func TestWatchDecksForPlaylistRefresh_RefreshesOnNotification(t *testing.T) { require.NotNil(t, queued) assert.True(t, queued.Refresh) - assert.Equal(t, "ZON-0123456789ab", queued.ID) + assert.Equal(t, "deck://0123456789ab", queued.ID) assert.Equal(t, "Weekend Plus", queued.Name) cancel() @@ -149,7 +152,7 @@ func TestWatchDecksForPlaylistRefresh_RefreshesOnNotification(t *testing.T) { func TestWatchDecksForPlaylistRefresh_IgnoresMalformedNotification(t *testing.T) { t.Parallel() svc := setupPlaylistTestEnv(t) - svc.State.SetActivePlaylist(playlists.NewPlaylist("ZON-0123456789ab", "Weekend", + svc.State.SetActivePlaylist(playlists.NewPlaylist("deck://0123456789ab", "Weekend", []playlists.PlaylistItem{{ZapScript: "**a"}})) bad := models.Notification{Method: models.NotificationDecksChanged, Params: []byte("not json")} @@ -163,3 +166,24 @@ func TestWatchDecksForPlaylistRefresh_IgnoresMalformedNotification(t *testing.T) default: } } + +// TestRefreshOpenDeckPlaylist_IgnoresAPlaylistNamedAfterTheDeck pins that a +// playlist is tied to a deck by having been opened from it, never by its ID. +// Any served playlist may call itself what a deck on this device opens as, +// and a change to that deck must not swap its items in. +func TestRefreshOpenDeckPlaylist_IgnoresAPlaylistNamedAfterTheDeck(t *testing.T) { + t.Parallel() + svc := setupPlaylistTestEnv(t) + served := playlists.NewPlaylist("deck://0123456789ab", "Somebody else's", + []playlists.PlaylistItem{{ZapScript: "**a"}}) + svc.State.SetActivePlaylist(served) + + notification := decksChangedNotification(t, "0123456789ab", models.DecksChangedUpdated) + refreshOpenDeckPlaylist(context.Background(), svc, ¬ification) + + select { + case queued := <-svc.PlaylistQueue: + t.Fatalf("unexpected playlist update queued: %+v", queued) + default: + } +} diff --git a/pkg/service/playlists/playlists.go b/pkg/service/playlists/playlists.go index 5990e08bd..38f59493b 100644 --- a/pkg/service/playlists/playlists.go +++ b/pkg/service/playlists/playlists.go @@ -34,11 +34,16 @@ type Playlist struct { // It is not part of playlist persistence or API responses. HoldToken *tokens.Token ID string - Name string - Slot string - Items []PlaylistItem - Index int - Playing bool + // DeckID names the deck on this device the playlist was opened from. It + // is set only by the deck loader, never from a served playlist, so it is + // what ties an open playlist to its deck; the ID is whatever its source + // chose to call it. + DeckID string + Name string + Slot string + Items []PlaylistItem + Index int + Playing bool // Clear signals the queue handler to remove the active playlist for this slot. Clear bool // Loop and LoopOne control end-of-playlist behavior set at load time. diff --git a/pkg/service/queues.go b/pkg/service/queues.go index 181a9dbd5..a67cadf15 100644 --- a/pkg/service/queues.go +++ b/pkg/service/queues.go @@ -752,7 +752,7 @@ func handlePlaylist( // The content of the active playlist changed underneath it, such as a // deck edited elsewhere: swap the items in and keep the position and // playback as they are. - if activePlaylist == nil || activePlaylist.ID != pls.ID { + if activePlaylist == nil || activePlaylist.ID != pls.ID || activePlaylist.DeckID != pls.DeckID { log.Debug().Str("id", pls.ID).Msg("playlist refresh ignored; it is not the active playlist") return } diff --git a/pkg/service/queues_playlist_test.go b/pkg/service/queues_playlist_test.go index f9552335c..ec5a3e22a 100644 --- a/pkg/service/queues_playlist_test.go +++ b/pkg/service/queues_playlist_test.go @@ -489,6 +489,12 @@ func TestHandlePlaylist_RefreshClampsIndexAndIgnoresOtherIDs(t *testing.T) { handlePlaylist(svc, &playlists.Playlist{ID: "other", Refresh: true, Items: active.Items}, nil) assert.Same(t, active, svc.State.GetActivePlaylist(), "a refresh for another playlist is ignored") + handlePlaylist(svc, &playlists.Playlist{ + ID: active.ID, DeckID: "0123456789ab", Refresh: true, Items: active.Items[:1], + }, nil) + assert.Same(t, active, svc.State.GetActivePlaylist(), + "a deck's refresh is ignored by a playlist that only shares its ID") + handlePlaylist(svc, &playlists.Playlist{ ID: active.ID, Refresh: true, Items: []playlists.PlaylistItem{{Name: "Only", ZapScript: "**only"}}, }, nil) diff --git a/pkg/testing/helpers/db_mocks.go b/pkg/testing/helpers/db_mocks.go index b2e1a9cbc..e5123dec9 100644 --- a/pkg/testing/helpers/db_mocks.go +++ b/pkg/testing/helpers/db_mocks.go @@ -359,6 +359,17 @@ func (m *MockUserDBI) UpsertRemoteDeck(deck *database.Deck) (bool, error) { return args.Bool(0), nil } +func (m *MockUserDBI) UpsertFetchedDeck(deck *database.Deck, maxCopies int) (database.FetchedDeckResult, error) { + args := m.Called(deck, maxCopies) + if err := args.Error(1); err != nil { + return database.FetchedDeckResult{}, fmt.Errorf("mock UserDBI upsert fetched deck failed: %w", err) + } + if result, ok := args.Get(0).(database.FetchedDeckResult); ok { + return result, nil + } + return database.FetchedDeckResult{}, nil +} + func (m *MockUserDBI) SetDeckItemAnchor(itemDBID int64, anchor *database.DeckItemAnchor) error { if err := m.Called(itemDBID, anchor).Error(0); err != nil { return fmt.Errorf("mock UserDBI set deck item anchor failed: %w", err) diff --git a/pkg/zapscript/commands.go b/pkg/zapscript/commands.go index 6be8d3cd1..165645da1 100644 --- a/pkg/zapscript/commands.go +++ b/pkg/zapscript/commands.go @@ -40,7 +40,6 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/boolutil" "github.com/ZaparooProject/zaparoo-core/v2/pkg/platforms" "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/decks" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/playlists" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/state" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/tokens" @@ -496,7 +495,6 @@ func RunCommand( // operation allowlist entirely. linkValue := "" linkOwned := false - trustedDeckID := "" if token.Source != tokens.SourceRemote { var linkErr error linkValue, linkOwned, linkErr = checkZapLink(cfg, pl, db, cmd) @@ -510,7 +508,12 @@ func RunCommand( if lenErr := ValidateScriptLength(linkValue); lenErr != nil { return platforms.CmdResult{}, fmt.Errorf("zap link error: %w", lenErr) } - linkValue = adoptZapLinkDeck(db, cmd.Args[0], linkValue) + // A playlist the account vouched for as the user's own is played as + // served: the user's copy of it is the deck they already hold, not a + // read-only one kept from the link. + if !linkOwned { + linkValue = adoptZapLinkDeck(db, cmd.Args[0], linkValue) + } log.Info().Msgf("valid zap link, replacing cmd: %s", linkValue) reader := zapscript.NewParser(linkValue) script, parseErr := reader.ParseScript() @@ -528,13 +531,10 @@ func RunCommand( cmd = script.Cmds[0] // A link the account vouched for as the user's own card or deck runs // trusted, the way a card the user wrote does; any other link body - // runs untrusted. - unsafe = !linkOwned - if linkOwned && cmd.Name == zapscript.ZapScriptCmdPlaylistOpen && len(cmd.Args) == 1 { - if deckID, isDeck := decks.ParseDeckURI(cmd.Args[0]); isDeck { - trustedDeckID = deckID - } - } + // runs untrusted. The answer only ever keeps trust, never restores + // it: whatever made this token untrusted chose to run the link, and + // owning the script does not make that choice the user's. + unsafe = token.Unsafe || !linkOwned } for i, arg := range cmd.Args { @@ -579,7 +579,6 @@ func RunCommand( PrepareMediaLaunch: opts.PrepareMediaLaunch, BeforeExit: opts.BeforeExit, RefreshOwnedDeck: opts.RefreshOwnedDeck, - TrustedDeckID: trustedDeckID, PlaybackManager: opts.PlaybackManager, LauncherCache: helpers.GlobalLauncherCache, UI: opts.UI, diff --git a/pkg/zapscript/commands_test.go b/pkg/zapscript/commands_test.go index 473853846..ec906e1b4 100644 --- a/pkg/zapscript/commands_test.go +++ b/pkg/zapscript/commands_test.go @@ -1075,3 +1075,78 @@ func TestRunCommandAppliesZapLinkForNonRemoteSource(t *testing.T) { assert.Contains(t, err.Error(), "unknown command") mockUserDB.AssertCalled(t, "GetZapLinkHost", "https://zaplink.example.com") } + +// ownedLinkServer answers every request as a credentialed link service that +// vouches for the link, and registers the credential it is sent. +func ownedLinkServer(t *testing.T, body string) (link string, db *database.Database) { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", MIMEZaparooZapScript) + w.Header().Set(HeaderZaparooOwned, "1") + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(server.Close) + config.SetAuthCfgForTesting(map[string]config.CredentialEntry{ + config.RemoteAuthLookupURL(server.URL): {Bearer: "zpd1_test"}, + }) + t.Cleanup(config.ClearAuthCfgForTesting) + + db, cleanup := testhelpers.NewTestDatabase(t) + t.Cleanup(cleanup) + require.NoError(t, db.UserDB.UpdateZapLinkHost(server.URL, 1)) + return server.URL + "/c1", db +} + +// TestRunCommandOwnedZapLinkNeverRestoresTrust pins that the owned answer is +// one-way. It keeps a tap the user made trusted; it never makes trusted a +// token that was already untrusted, such as an item of somebody else's deck +// that names the user's own card. Owning a script says who wrote it, not who +// chose to run it. +func TestRunCommandOwnedZapLinkNeverRestoresTrust(t *testing.T) { + // No t.Parallel(): the auth config is global. + link, db := ownedLinkServer(t, "**input.keyboard:a?when=false") + mockPlatform := mocks.NewMockPlatform() + mockPlatform.On("ID").Return("test") + + run := func(token tokens.Token) platforms.CmdResult { + result, err := RunCommand( + t.Context(), mockPlatform, &config.Instance{}, playlists.PlaylistController{}, token, + zapscript.Command{Name: "launch", Args: []string{link}}, + 1, 0, db, &RunCommandOptions{}, &zapscript.ArgExprEnv{}, + ) + require.NoError(t, err) + return result + } + assert.False(t, run(tokens.Token{Source: tokens.SourceReader}).Unsafe, + "the user's own tap on their own card runs trusted") + assert.True(t, run(tokens.Token{Source: tokens.SourcePlaylist, Unsafe: true}).Unsafe, + "an untrusted token stays untrusted whoever owns the link it names") +} + +// TestRunCommandOwnedDeckZapLinkPlaysAsServed pins that a playlist the +// account vouched for runs trusted as it was served, and is not kept as a +// read-only copy beside the deck the user already holds. +func TestRunCommandOwnedDeckZapLinkPlaysAsServed(t *testing.T) { + // No t.Parallel(): the auth config is global. + link, db := ownedLinkServer(t, + `**playlist.open:{"id":"ZON-LHM6N9T8","name":"Mine","items":[{"name":"A","zapscript":"**input.keyboard:a"}]}`) + + queue := make(chan *playlists.Playlist, 1) + result, err := RunCommand( + t.Context(), newPlaylistTestPlatform(), &config.Instance{}, + playlists.PlaylistController{Queue: queue}, + tokens.Token{Source: tokens.SourceReader}, + zapscript.Command{Name: "launch", Args: []string{link}}, + 1, 0, db, &RunCommandOptions{}, &zapscript.ArgExprEnv{}, + ) + require.NoError(t, err) + assert.False(t, result.Unsafe) + + pls := <-queue + assert.Equal(t, "ZON-LHM6N9T8", pls.ID) + assert.Empty(t, pls.DeckID, "the served playlist runs as served") + assert.False(t, pls.Unsafe, "the user's own deck runs its items trusted") + all, err := db.UserDB.ListDecks() + require.NoError(t, err) + assert.Empty(t, all, "no read-only copy of the user's own deck is kept") +} diff --git a/pkg/zapscript/playlist_deck.go b/pkg/zapscript/playlist_deck.go index e7f6b1c18..440c10c9b 100644 --- a/pkg/zapscript/playlist_deck.go +++ b/pkg/zapscript/playlist_deck.go @@ -47,27 +47,27 @@ const ( // ErrDeckNotFound reports a deck:// argument naming no deck on this device. var ErrDeckNotFound = errors.New("deck not found") -// adoptZapLinkDeck recognizes a ZapLink body that serves a deck and caches -// the deck locally, returning a script that opens the local copy instead. A -// deck opened this way plays offline next time, and a deck this device owns -// opens its own editable copy. Any other body is returned unchanged. +// adoptZapLinkDeck recognizes a ZapLink body that serves a playlist and keeps +// it as a deck on this device, returning the served command with its playlist +// swapped for the local copy. A deck kept this way plays offline next time, +// shows in the deck list and has its games tagged. Only the body decides: any +// host and any link that serves a playlist is kept the same way. Any other +// body, and one that cannot be stored, is returned unchanged. func adoptZapLinkDeck(db *database.Database, link, body string) string { if db == nil || db.UserDB == nil { return body } - deckID, ok := decks.DeckIDFromZapLinkURL(link) + cmd, arg, ok := decks.ParseServedPlaylist(body) if !ok { return body } - arg, ok := decks.ParseDeckPlaylist(body, deckID) - if !ok { - return body - } - if err := decks.StoreFetchedDeck(db, link, deckID, &arg); err != nil { - log.Warn().Err(err).Str("deck", deckID).Msg("failed to cache deck from zap link; playing it as served") + deckID, err := decks.StoreFetchedDeck(db, link, &arg) + if err != nil { + log.Warn().Err(err).Msg("failed to keep deck from zap link; playing it as served") return body } - return "**" + zapscript.ZapScriptCmdPlaylistOpen + ":" + decks.DeckURI(deckID) + cmd.Args = []string{decks.DeckURI(deckID)} + return cmd.String() } // loadDeckPlaylist builds a playlist from a deck on this device, refreshing @@ -104,12 +104,12 @@ func loadDeckPlaylist( rand.Shuffle(len(items), func(i, j int) { items[i], items[j] = items[j], items[i] }) } - pls := playlists.NewPlaylist(decks.PlaylistID(deck.DeckID), deck.Name, items) + pls := playlists.NewPlaylist(decks.PlaylistID(deck), deck.Name, items) + pls.DeckID = deck.DeckID // Only the user's own decks are trusted. A cached copy of somebody // else's deck runs its items the way its ZapLink would, whatever script - // opened it, unless the account vouched for this very link as the - // user's own deck. - pls.Unsafe = !deck.Owned && env.TrustedDeckID != deck.DeckID + // opened it. + pls.Unsafe = !deck.Owned slot, slotErr := mediaslot.Normalize(env.Cmd.AdvArgs.Get(zapscript.KeySlot)) if slotErr != nil { return nil, fmt.Errorf("normalize media slot: %w", slotErr) @@ -166,12 +166,12 @@ func refreshDeck(pl platforms.Platform, env *platforms.CmdEnv, deck *database.De log.Debug().Err(err).Str("deck", deck.DeckID).Msg("deck refresh skipped; opening cached copy") return nil } - arg, ok := decks.ParseDeckPlaylist(string(body), deck.DeckID) + _, arg, ok := decks.ParseServedPlaylist(string(body)) if !ok { log.Debug().Str("deck", deck.DeckID).Msg("deck source no longer serves the deck; opening cached copy") return nil } - if storeErr := decks.StoreFetchedDeck(env.Database, deck.SourceURL, deck.DeckID, &arg); storeErr != nil { + if _, storeErr := decks.StoreFetchedDeck(env.Database, deck.SourceURL, &arg); storeErr != nil { log.Debug().Err(storeErr).Str("deck", deck.DeckID).Msg("failed to store refreshed deck") return nil } diff --git a/pkg/zapscript/playlist_deck_test.go b/pkg/zapscript/playlist_deck_test.go index d86a2c2fc..c174ecc2d 100644 --- a/pkg/zapscript/playlist_deck_test.go +++ b/pkg/zapscript/playlist_deck_test.go @@ -55,9 +55,17 @@ func deckTestEnv(t *testing.T, arg string) (platforms.CmdEnv, *database.Database }, db, queue } +// The links a deck is reached through in these tests. One is a real link on +// the hosted service and the other belongs to nobody in particular: nothing +// about a host or a link's shape may decide how its playlist is treated. +const ( + hostedDeckLink = "https://zpr.au/d$lhm6n9t8" + thirdPartyDeckLink = "https://decks.example/shelf/anything/at/all?ref=1" +) + func servedDeckBody(t *testing.T, id, name string, items ...decks.PlaylistArgItem) string { t.Helper() - encoded, err := json.Marshal(decks.PlaylistArg{ID: "ZON-" + id, Name: name, Items: items}) + encoded, err := json.Marshal(decks.PlaylistArg{ID: id, Name: name, Items: items}) require.NoError(t, err) return "**playlist.open:" + string(encoded) } @@ -82,7 +90,8 @@ func TestCmdPlaylistLoad_Deck(t *testing.T) { require.NoError(t, err) assert.True(t, result.PlaylistChanged) pls := <-queue - assert.Equal(t, "ZON-0123456789ab", pls.ID, "a deck opens under the playlist ID its link uses") + assert.Equal(t, "deck://0123456789ab", pls.ID, "a deck made here opens as its own URI") + assert.Equal(t, "0123456789ab", pls.DeckID) assert.Equal(t, "Weekend", pls.Name) require.Len(t, pls.Items, 2) assert.Equal(t, "**launch.system:NES", pls.Items[0].ZapScript) @@ -103,12 +112,6 @@ func TestCmdPlaylistLoad_NotOwnedDeckIsUntrusted(t *testing.T) { require.NoError(t, err) pls := <-queue assert.True(t, pls.Unsafe, "a cached copy of somebody else's deck runs its items untrusted") - - env.TrustedDeckID = "0123456789ab" - _, err = cmdPlaylistLoad(newPlaylistTestPlatform(), env) - require.NoError(t, err) - pls = <-queue - assert.False(t, pls.Unsafe, "unless the account vouched for this link as the user's own deck") } func TestCmdPlaylistLoad_DeckNotFound(t *testing.T) { @@ -118,37 +121,99 @@ func TestCmdPlaylistLoad_DeckNotFound(t *testing.T) { require.ErrorIs(t, err, ErrDeckNotFound) } +// Any link that serves a playlist is kept as a deck, whoever hosts it and +// whatever the link or the playlist's ID look like. func TestAdoptZapLinkDeck(t *testing.T) { + t.Parallel() + for name, tc := range map[string]struct{ link, playlistID string }{ + "hosted": {link: hostedDeckLink, playlistID: "ZON-LHM6N9T8"}, + "third party": {link: thirdPartyDeckLink, playlistID: "party-list"}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + db, cleanup := testhelpers.NewTestDatabase(t) + t.Cleanup(cleanup) + body := servedDeckBody(t, tc.playlistID, "Theirs", + decks.PlaylistArgItem{Name: "Game", ZapScript: "**launch.title:SNES/Game"}) + + rewritten := adoptZapLinkDeck(db, tc.link, body) + all, err := db.UserDB.ListDecks() + require.NoError(t, err) + require.Len(t, all, 1) + parsed, err := zapscript.NewParser(rewritten).ParseScript() + require.NoError(t, err) + require.Len(t, parsed.Cmds, 1) + assert.Equal(t, zapscript.ZapScriptCmdPlaylistOpen, parsed.Cmds[0].Name) + assert.Equal(t, []string{decks.DeckURI(all[0].DeckID)}, parsed.Cmds[0].Args) + stored, err := db.UserDB.GetDeck(all[0].DeckID) + require.NoError(t, err) + assert.False(t, stored.Owned) + assert.Equal(t, tc.link, stored.SourceURL) + assert.Equal(t, tc.playlistID, decks.PlaylistID(stored), "the copy opens as the playlist it was served as") + require.Len(t, stored.Items, 1) + + assert.Equal(t, rewritten, adoptZapLinkDeck(db, tc.link, body), "the same link is the same deck") + all, err = db.UserDB.ListDecks() + require.NoError(t, err) + assert.Len(t, all, 1) + }) + } +} + +func TestAdoptZapLinkDeck_KeepsTheServedCommand(t *testing.T) { t.Parallel() db, cleanup := testhelpers.NewTestDatabase(t) t.Cleanup(cleanup) - body := servedDeckBody(t, "0123456789AB", "Theirs", - decks.PlaylistArgItem{Name: "Game", ZapScript: "**launch.title:SNES/Game"}) + encoded, err := json.Marshal(decks.PlaylistArg{ID: "p", Name: "Mix", Items: []decks.PlaylistArgItem{ + {Name: "A", ZapScript: "**launch.system:SNES"}, + }}) + require.NoError(t, err) - rewritten := adoptZapLinkDeck(db, "https://zpr.au/d0123456789ab", body) - assert.Equal(t, "**playlist.open:deck://0123456789ab", rewritten) - stored, err := db.UserDB.GetDeck("0123456789ab") + rewritten := adoptZapLinkDeck(db, thirdPartyDeckLink, "**playlist.play:"+string(encoded)+"?mode=shuffle") + parsed, err := zapscript.NewParser(rewritten).ParseScript() + require.NoError(t, err) + require.Len(t, parsed.Cmds, 1) + assert.Equal(t, zapscript.ZapScriptCmdPlaylistPlay, parsed.Cmds[0].Name, "a playlist served to play still plays") + assert.Equal(t, "shuffle", parsed.Cmds[0].AdvArgs.Get(zapscript.KeyMode)) + require.Len(t, parsed.Cmds[0].Args, 1) + _, isDeck := decks.ParseDeckURI(parsed.Cmds[0].Args[0]) + assert.True(t, isDeck) +} + +func TestAdoptZapLinkDeck_OtherBodiesPassThrough(t *testing.T) { + t.Parallel() + db, cleanup := testhelpers.NewTestDatabase(t) + t.Cleanup(cleanup) + for _, body := range []string{ + "**launch.system:SNES", + "**playlist.open:/roms/list.pls", + servedDeckBody(t, "p", "Two") + "||**stop", + servedDeckBody(t, "p", "Nothing to run"), + } { + assert.Equal(t, body, adoptZapLinkDeck(db, hostedDeckLink, body)) + } + all, err := db.UserDB.ListDecks() require.NoError(t, err) - assert.False(t, stored.Owned) - assert.Equal(t, "https://zpr.au/d0123456789ab", stored.SourceURL) - require.Len(t, stored.Items, 1) - - // Anything else passes through untouched. - assert.Equal(t, body, adoptZapLinkDeck(db, "https://zpr.au/c0123456789ab", body), "a card link is not a deck") - assert.Equal(t, "**launch.system:SNES", - adoptZapLinkDeck(db, "https://zpr.au/d0123456789ab", "**launch.system:SNES")) - other := servedDeckBody(t, "zzzzzzzzzzzz", "Other") - assert.Equal(t, other, adoptZapLinkDeck(db, "https://zpr.au/d0123456789ab", other), - "a served playlist for another ID is not this deck") - - // A deck this device owns keeps its own copy and still opens locally. + assert.Empty(t, all) +} + +// A served playlist never reaches a deck the user owns, whatever the link or +// the playlist calls itself: it is kept as its own read-only copy. +func TestAdoptZapLinkDeck_NeverResolvesToAnOwnedDeck(t *testing.T) { + t.Parallel() + db, cleanup := testhelpers.NewTestDatabase(t) + t.Cleanup(cleanup) require.NoError(t, db.UserDB.CreateDeck(&database.Deck{DeckID: "aaaaaaaaaaaa", Name: "Mine", Owned: true})) - mineBody := servedDeckBody(t, "AAAAAAAAAAAA", "Server copy") - assert.Equal(t, "**playlist.open:deck://aaaaaaaaaaaa", - adoptZapLinkDeck(db, "https://zpr.au/daaaaaaaaaaaa", mineBody)) + + body := servedDeckBody(t, "deck://aaaaaaaaaaaa", "Imposter", + decks.PlaylistArgItem{Name: "A", ZapScript: "**input.keyboard:a"}) + rewritten := adoptZapLinkDeck(db, "https://decks.example/daaaaaaaaaaaa", body) + assert.NotContains(t, rewritten, "aaaaaaaaaaaa") + mine, err := db.UserDB.GetDeck("aaaaaaaaaaaa") require.NoError(t, err) assert.Equal(t, "Mine", mine.Name) + assert.Empty(t, mine.Items) } // A cached copy of somebody else's deck is fetched again before it opens, @@ -187,7 +252,7 @@ func TestRefreshDeck_CachedCopy(t *testing.T) { stale.FetchedAt = 0 assert.Nil(t, refreshDeck(newPlaylistTestPlatform(), &env, stale), "a failed fetch opens the cached copy") - serve.Store(servedDeckBody(t, "0123456789AB", "New", + serve.Store(servedDeckBody(t, "party-list", "New", decks.PlaylistArgItem{Name: "B", ZapScript: "**b"}, decks.PlaylistArgItem{Name: "C", ZapScript: "**c"})) refreshed := refreshDeck(newPlaylistTestPlatform(), &env, stale) require.NotNil(t, refreshed) @@ -195,7 +260,7 @@ func TestRefreshDeck_CachedCopy(t *testing.T) { require.Len(t, refreshed.Items, 2) cached, err := db.UserDB.GetZapLinkCache(server.URL) require.NoError(t, err) - assert.Contains(t, cached, `"ZON-0123456789AB"`, "the link cache follows the refresh for offline taps") + assert.Contains(t, cached, `"party-list"`, "the link cache follows the refresh for offline taps") } // A stored source that is not a ZapLink host is never fetched, so a user @@ -207,7 +272,7 @@ func TestRefreshDeck_SourceMustBeAZapLink(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { hits.Add(1) w.Header().Set("Content-Type", MIMEZaparooZapScript) - _, _ = w.Write([]byte(servedDeckBody(t, "0123456789AB", "Theirs", + _, _ = w.Write([]byte(servedDeckBody(t, "party-list", "Theirs", decks.PlaylistArgItem{Name: "B", ZapScript: "**b"}))) })) t.Cleanup(server.Close) @@ -249,32 +314,68 @@ func (t deckZapLinkTransport) RoundTrip(req *http.Request) (*http.Response, erro }, nil } -// TestRunCommand_OwnDeckZapLinkStaysTrusted pins that the owner tapping a -// card for a deck this device owns runs its items trusted, so an input or -// execute item in the user's own deck keeps working. The card carries a -// ZapLink, so the token is flagged unsafe by the fetch; ownership of the deck -// it resolves to, not the trust of the link, decides how its items run. -// -// Deriving the playlist's trust from the link as well would break this while -// gaining nothing: an untrusted script naming an owned deck can only play the -// user's own items, and a fetched deck can never overwrite an owned one. -func TestRunCommand_OwnDeckZapLinkStaysTrusted(t *testing.T) { - const deckID = "0123456789ab" - link := "https://zpr.au/d" + deckID - body := servedDeckBody(t, deckID, "Mine", - decks.PlaylistArgItem{Name: "A", ZapScript: "**input.keyboard:a"}) - +func useDeckZapLinkTransport(t *testing.T, transport http.RoundTripper) { + t.Helper() oldWellKnown, oldZap := currentWellKnownFetchClient(), currentZapFetchClient() t.Cleanup(func() { zapFetchClientMu.Lock() wellKnownFetchClient, zapFetchClient = oldWellKnown, oldZap zapFetchClientMu.Unlock() }) - transport := deckZapLinkTransport{body: body} zapFetchClientMu.Lock() wellKnownFetchClient = newWellKnownFetchClient(transport) zapFetchClient = newZapFetchClient(transport) zapFetchClientMu.Unlock() +} + +// TestRunCommand_DeckZapLinkIsKeptFromAnyHost pins the whole tap: a link on +// the hosted service, in the shape it really has, and a link on somebody +// else's host both end up as a read-only deck that opens untrusted. +func TestRunCommand_DeckZapLinkIsKeptFromAnyHost(t *testing.T) { + for name, tc := range map[string]struct{ link, playlistID string }{ + "hosted": {link: hostedDeckLink, playlistID: "ZON-LHM6N9T8"}, + "third party": {link: thirdPartyDeckLink, playlistID: "party-list"}, + } { + t.Run(name, func(t *testing.T) { + useDeckZapLinkTransport(t, deckZapLinkTransport{body: servedDeckBody(t, tc.playlistID, "Theirs", + decks.PlaylistArgItem{Name: "A", ZapScript: "**input.keyboard:a"})}) + db, cleanup := testhelpers.NewTestDatabase(t) + t.Cleanup(cleanup) + + queue := make(chan *playlists.Playlist, 1) + result, err := RunCommand( + t.Context(), newPlaylistTestPlatform(), &config.Instance{}, + playlists.PlaylistController{Queue: queue}, + tokens.Token{Text: tc.link}, + zapscript.Command{Name: zapscript.ZapScriptCmdLaunch, Args: []string{tc.link}}, + 1, 0, db, &RunCommandOptions{}, &zapscript.ArgExprEnv{}, + ) + require.NoError(t, err) + assert.True(t, result.Unsafe, "a fetched ZapLink flags the token unsafe") + + all, err := db.UserDB.ListDecks() + require.NoError(t, err) + require.Len(t, all, 1, "the served playlist is kept as a deck") + assert.False(t, all[0].Owned) + assert.Equal(t, tc.link, all[0].SourceURL) + + pls := <-queue + assert.Equal(t, tc.playlistID, pls.ID, "the copy opens as the playlist it was served as") + assert.Equal(t, all[0].DeckID, pls.DeckID) + assert.True(t, pls.Unsafe, "somebody else's deck runs its items untrusted") + }) + } +} + +// TestRunCommand_DeckZapLinkNeverOpensAnOwnedDeck pins that nothing a link +// serves can steer a tap into a trusted open of a deck the user owns. Only +// the account vouching for the link makes it trusted, and that is decided by +// who answered, never by what the link or the playlist is called. +func TestRunCommand_DeckZapLinkNeverOpensAnOwnedDeck(t *testing.T) { + const deckID = "0123456789ab" + link := "https://decks.example/d" + deckID + useDeckZapLinkTransport(t, deckZapLinkTransport{body: servedDeckBody(t, "deck://"+deckID, "Imposter", + decks.PlaylistArgItem{Name: "A", ZapScript: "**input.keyboard:a"})}) db, cleanup := testhelpers.NewTestDatabase(t) t.Cleanup(cleanup) @@ -286,7 +387,7 @@ func TestRunCommand_OwnDeckZapLinkStaysTrusted(t *testing.T) { })) queue := make(chan *playlists.Playlist, 1) - result, err := RunCommand( + _, err := RunCommand( t.Context(), newPlaylistTestPlatform(), &config.Instance{}, playlists.PlaylistController{Queue: queue}, tokens.Token{Text: link}, @@ -294,9 +395,9 @@ func TestRunCommand_OwnDeckZapLinkStaysTrusted(t *testing.T) { 1, 0, db, &RunCommandOptions{}, &zapscript.ArgExprEnv{}, ) require.NoError(t, err) - assert.True(t, result.Unsafe, "a fetched ZapLink flags the token unsafe") pls := <-queue - assert.Equal(t, decks.PlaylistID(deckID), pls.ID, "the link resolves to the local owned deck") - assert.False(t, pls.Unsafe, "the user's own deck runs trusted however its card reached the device") + assert.NotEqual(t, deckID, pls.DeckID, "the link does not resolve to the owned deck") + assert.Equal(t, "Imposter", pls.Name) + assert.True(t, pls.Unsafe) } diff --git a/pkg/zapscript/zaplinks.go b/pkg/zapscript/zaplinks.go index 9a333cff9..6524c996a 100644 --- a/pkg/zapscript/zaplinks.go +++ b/pkg/zapscript/zaplinks.go @@ -270,12 +270,12 @@ func isKnownZapLinkHost(link string, db *database.Database) bool { if err != nil || validateZapLinkURL(u) != nil { return false } - _, supported, err := db.UserDB.GetZapLinkHost(u.Scheme + "://" + u.Host) + supported, found, err := db.UserDB.GetZapLinkHost(u.Scheme + "://" + u.Host) if err != nil { log.Debug().Err(err).Msg("error checking db for zap link host") return false } - return supported + return found && supported } // HeaderZaparooOwned is set by a link service on a response to a request @@ -358,7 +358,14 @@ func getRemoteZapScriptOwned(parent context.Context, urlStr, platform string) (b return nil, false, errors.New("invalid content type") } - owned = resp.Header.Get(HeaderZaparooOwned) == "1" && credentialSentTo(urlStr) + // The host that answered is the one that must have been sent the + // credential. After a redirect that is not the host the link names, and + // a host that never saw the credential cannot vouch for anything. + answeredBy := urlStr + if resp.Request != nil && resp.Request.URL != nil { + answeredBy = resp.Request.URL.String() + } + owned = resp.Header.Get(HeaderZaparooOwned) == "1" && credentialSentTo(answeredBy) log.Debug().Int("size", len(body)).Bool("owned", owned).Msg("received zap link body") return body, owned, nil diff --git a/pkg/zapscript/zaplinks_test.go b/pkg/zapscript/zaplinks_test.go index becee366f..37093d3ca 100644 --- a/pkg/zapscript/zaplinks_test.go +++ b/pkg/zapscript/zaplinks_test.go @@ -1112,3 +1112,51 @@ func zapLinkCertPEM(t *testing.T, cert *x509.Certificate) []byte { Bytes: cert.Raw, }) } + +// TestGetRemoteZapScriptOwnedIgnoresARedirectedClaim pins that only the host +// that was sent the credential can vouch. A credentialed host that redirects +// elsewhere hands the answer to a host that never saw the credential, and +// that host cannot know who asked. +func TestGetRemoteZapScriptOwnedIgnoresARedirectedClaim(t *testing.T) { + // No t.Parallel(): the auth config is global. + var sawCredential atomic.Bool + elsewhere := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sawCredential.Store(r.Header.Get("Authorization") != "") + w.Header().Set("Content-Type", MIMEZaparooZapScript) + w.Header().Set(HeaderZaparooOwned, "1") + _, _ = w.Write([]byte("**input.keyboard:a")) + })) + defer elsewhere.Close() + linked := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, elsewhere.URL+"/c1", http.StatusFound) + })) + defer linked.Close() + config.SetAuthCfgForTesting(map[string]config.CredentialEntry{ + config.RemoteAuthLookupURL(linked.URL): {Bearer: "zpd1_test"}, + }) + t.Cleanup(config.ClearAuthCfgForTesting) + + body, owned, err := getRemoteZapScriptOwned(context.Background(), linked.URL+"/c1", "test") + require.NoError(t, err) + assert.Equal(t, "**input.keyboard:a", string(body)) + assert.False(t, sawCredential.Load(), "the credential stays with the host it belongs to") + assert.False(t, owned, "a host that was sent no credential cannot vouch for ownership") +} + +// TestIsKnownZapLinkHost pins the gate a stored address passes before it is +// fetched: only a host this device learned serves ZapScript, never one it +// learned does not, and never one it has not met. +func TestIsKnownZapLinkHost(t *testing.T) { + t.Parallel() + db, cleanup := testhelpers.NewTestDatabase(t) + t.Cleanup(cleanup) + require.NoError(t, db.UserDB.UpdateZapLinkHost("https://serves.example", 1)) + require.NoError(t, db.UserDB.UpdateZapLinkHost("https://refuses.example", 0)) + + assert.True(t, isKnownZapLinkHost("https://serves.example/any/path", db)) + assert.False(t, isKnownZapLinkHost("https://refuses.example/any/path", db), + "a host recorded as not serving ZapScript is never fetched") + assert.False(t, isKnownZapLinkHost("https://stranger.example/any/path", db)) + assert.False(t, isKnownZapLinkHost("http://serves.example/any/path", db), "a public link must be HTTPS") + assert.False(t, isKnownZapLinkHost("https://serves.example/any/path", nil)) +} From 33c4ccf6ad528ef9b7caa3f64e087d199f327caf Mon Sep 17 00:00:00 2001 From: sim <callan@wizzo.au> Date: Sat, 19 Sep 2026 14:58:28 +0800 Subject: [PATCH 3/3] fix(zapscript): ignore an owned answer reached through a redirect - The credential is attached per hop, so a link on any host could redirect to a host this device is linked to and borrow its honest vouch for a card the tapped link never named. The owned answer now counts only when no redirect was followed; a redirected body is still served, untrusted. --- pkg/zapscript/zaplinks.go | 23 +++++++++++----------- pkg/zapscript/zaplinks_test.go | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/pkg/zapscript/zaplinks.go b/pkg/zapscript/zaplinks.go index 6524c996a..afacf1914 100644 --- a/pkg/zapscript/zaplinks.go +++ b/pkg/zapscript/zaplinks.go @@ -297,9 +297,10 @@ func getRemoteZapScriptContext(parent context.Context, urlStr, platform string) // getRemoteZapScriptOwned fetches a ZapLink body and reports whether the // link service said the linked account owns what the link names. The answer -// is trusted only from a host this device sent its own credential to: a -// host that got no credential cannot know who asked, so its claim is -// ignored. Absent means not owned. +// is trusted only from a host this device sent its own credential to, and +// only when that host is the one the link names: a host that got no +// credential cannot know who asked, and an answer reached through a redirect +// is not for this link. Absent means not owned. func getRemoteZapScriptOwned(parent context.Context, urlStr, platform string) (body []byte, owned bool, err error) { ctx, cancel := context.WithTimeout(parent, 10*time.Second) defer cancel() @@ -358,14 +359,14 @@ func getRemoteZapScriptOwned(parent context.Context, urlStr, platform string) (b return nil, false, errors.New("invalid content type") } - // The host that answered is the one that must have been sent the - // credential. After a redirect that is not the host the link names, and - // a host that never saw the credential cannot vouch for anything. - answeredBy := urlStr - if resp.Request != nil && resp.Request.URL != nil { - answeredBy = resp.Request.URL.String() - } - owned = resp.Header.Get(HeaderZaparooOwned) == "1" && credentialSentTo(answeredBy) + // The answer only counts for the link that was tapped. Once a redirect + // is followed, whoever wrote the link chose where it led: a host that + // never saw the credential cannot vouch, and a linked host reached that + // way vouches for a card the link did not name. The body is still + // served; it just runs untrusted. Request.Response is set only on a + // request the client made to follow a redirect. + redirected := resp.Request != nil && resp.Request.Response != nil + owned = resp.Header.Get(HeaderZaparooOwned) == "1" && !redirected && credentialSentTo(urlStr) log.Debug().Int("size", len(body)).Bool("owned", owned).Msg("received zap link body") return body, owned, nil diff --git a/pkg/zapscript/zaplinks_test.go b/pkg/zapscript/zaplinks_test.go index 37093d3ca..88aa911d2 100644 --- a/pkg/zapscript/zaplinks_test.go +++ b/pkg/zapscript/zaplinks_test.go @@ -1143,6 +1143,41 @@ func TestGetRemoteZapScriptOwnedIgnoresARedirectedClaim(t *testing.T) { assert.False(t, owned, "a host that was sent no credential cannot vouch for ownership") } +// TestGetRemoteZapScriptOwnedIgnoresAVouchReachedByRedirect pins that the +// owned answer only counts for the link that was tapped. A link on any host +// may redirect to a host this device is linked to, which then vouches +// honestly for the user's own card; but the link named somewhere else, and +// whoever wrote it chose where it led, so the body runs untrusted. +func TestGetRemoteZapScriptOwnedIgnoresAVouchReachedByRedirect(t *testing.T) { + // No t.Parallel(): the auth config is global. + var sawCredential atomic.Bool + linked := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sawCredential.Store(r.Header.Get("Authorization") != "") + w.Header().Set("Content-Type", MIMEZaparooZapScript) + w.Header().Set(HeaderZaparooOwned, "1") + _, _ = w.Write([]byte("**input.keyboard:a")) + })) + defer linked.Close() + elsewhere := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, linked.URL+"/c1", http.StatusFound) + })) + defer elsewhere.Close() + config.SetAuthCfgForTesting(map[string]config.CredentialEntry{ + config.RemoteAuthLookupURL(linked.URL): {Bearer: "zpd1_test"}, + }) + t.Cleanup(config.ClearAuthCfgForTesting) + + _, owned, err := getRemoteZapScriptOwned(context.Background(), linked.URL+"/c1", "test") + require.NoError(t, err) + require.True(t, owned, "the linked host vouches for its own link") + + body, owned, err := getRemoteZapScriptOwned(context.Background(), elsewhere.URL+"/x", "test") + require.NoError(t, err) + assert.Equal(t, "**input.keyboard:a", string(body), "the redirected body is still served") + assert.True(t, sawCredential.Load(), "the linked host was sent its credential and vouched") + assert.False(t, owned, "a vouch reached through a redirect is not for the link that was tapped") +} + // TestIsKnownZapLinkHost pins the gate a stored address passes before it is // fetched: only a host this device learned serves ZapScript, never one it // learned does not, and never one it has not met.