diff --git a/internal/dinners/dinners.go b/internal/dinners/dinners.go index 7a39832..9dbaddc 100644 --- a/internal/dinners/dinners.go +++ b/internal/dinners/dinners.go @@ -496,10 +496,19 @@ func checkRecipeURL(raw string) (*url.URL, error) { return u, nil } +// credentialWords end the name nameFromURL takes from a slug: a pasted link +// can carry a personal token in its path, as in /stew-access_token-abc123, +// and the name is shown in every dinner response. +var credentialWords = map[string]bool{ + "access": true, "api": true, "apikey": true, "auth": true, "credential": true, + "credentials": true, "passcode": true, "password": true, "passwd": true, + "secret": true, "session": true, "sig": true, "signature": true, "token": true, +} + // nameFromURL names a dinner from the last path segment of u: separators -// become spaces, a file extension and words of only digits are dropped and -// the first letter is capitalized, so -// https://example.com/recipes/12345/chicken-tikka-masala/ is +// become spaces, a file extension and words holding a digit are dropped, the +// name ends at the first of credentialWords and the first letter is +// capitalized, so https://example.com/recipes/12345/chicken-tikka-masala/ is // "Chicken tikka masala". ok is false when no usable name is left. func nameFromURL(u *url.URL) (name string, ok bool) { segments := strings.FieldsFunc(u.Path, func(r rune) bool { return r == '/' }) @@ -512,8 +521,12 @@ func nameFromURL(u *url.URL) (name string, ok bool) { } var words []string for _, w := range strings.FieldsFunc(slug, func(r rune) bool { return r == '-' || r == '_' || r == '+' || unicode.IsSpace(r) }) { - if strings.IndexFunc(w, func(r rune) bool { return !unicode.IsDigit(r) }) >= 0 { - words = append(words, strings.ToLower(w)) + w = strings.ToLower(w) + if credentialWords[w] { + break + } + if strings.IndexFunc(w, unicode.IsDigit) < 0 { + words = append(words, w) } } name = strings.Join(words, " ") diff --git a/internal/dinners/dinners_test.go b/internal/dinners/dinners_test.go index 9f8bd80..accd4b1 100644 --- a/internal/dinners/dinners_test.go +++ b/internal/dinners/dinners_test.go @@ -19,6 +19,13 @@ func TestNameFromURL(t *testing.T) { {"https://example.com/", ""}, {"https://example.com/recipe/98765", ""}, {"https://example.com/the-best-easy-recipe", ""}, + {"https://example.com/best-chili-v2", "Best chili"}, + {"https://example.com/stew-access_token-personaltokenabc123", "Stew"}, + {"https://example.com/stew-token-abcdefghijk", "Stew"}, + {"https://example.com/chili-api-key-zyxwvut", "Chili"}, + {"https://example.com/stew-x7k2p9q4", "Stew"}, + {"https://example.com/recipes/token-abcdefghijk", ""}, + {"https://example.com/key-lime-pie", "Key lime pie"}, } for _, tt := range tests { u, err := url.Parse(tt.link) diff --git a/internal/server/dinners_test.go b/internal/server/dinners_test.go index a8285e2..b956266 100644 --- a/internal/server/dinners_test.go +++ b/internal/server/dinners_test.go @@ -266,6 +266,36 @@ func TestDinnerResponsesHideRecipeURL(t *testing.T) { } } +// TestDinnerResponsesHideTokenInLinkPath: a personal token can sit in the +// link's last path segment, which names the dinner, so the name stops before +// it and neither the create response nor a later list read shows it. +func TestDinnerResponsesHideTokenInLinkPath(t *testing.T) { + t.Parallel() + router, _ := newDinnersRouter(t) + + const link = "https://c.example/recipes/stew-access_token-personaltokenabc123" + rec, body := callDinners(t, router, http.MethodPost, dinnersPath, `{"recipe_url":"`+link+`"}`) + if rec.Code != http.StatusCreated { + t.Fatalf("status = %d, want 201: %s", rec.Code, rec.Body) + } + if body["name"] != "Stew" { + t.Errorf("name = %v, want Stew", body["name"]) + } + created := rec.Body.String() + rec, _ = callDinners(t, router, http.MethodGet, dinnersPath+"?status=regular", "") + if rec.Code != http.StatusOK { + t.Fatalf("GET status=regular = %d: %s", rec.Code, rec.Body) + } + for _, resp := range []struct{ name, body string }{{"create", created}, {"list", rec.Body.String()}} { + lower := strings.ToLower(resp.body) + for _, secret := range []string{"personaltokenabc123", "abc123", "access", "token"} { + if strings.Contains(lower, secret) { + t.Errorf("%s response contains %q: %s", resp.name, secret, resp.body) + } + } + } +} + func TestCreateDinnerDuplicate(t *testing.T) { t.Parallel() router, st := newDinnersRouter(t) diff --git a/web/src/sw.ts b/web/src/sw.ts index ca712ff..8e797c9 100644 --- a/web/src/sw.ts +++ b/web/src/sw.ts @@ -28,7 +28,7 @@ declare const self: ServiceWorkerGlobalScope & { __WB_MANIFEST: Array<{ url: string; revision: string | null }> } -// The precache becomes mealquest-precache-v1. +// The precache becomes mealquest-precache-v2. setCacheNameDetails({ prefix: 'mealquest', precache: 'precache', suffix: cacheVersion }) const day = 24 * 60 * 60 diff --git a/web/src/sw/routes.test.ts b/web/src/sw/routes.test.ts index 9b57d20..273df82 100644 --- a/web/src/sw/routes.test.ts +++ b/web/src/sw/routes.test.ts @@ -146,11 +146,20 @@ describe('cache names', () => { it('all start with mealquest-', () => { for (const name of Object.values(cacheNames)) expect(name).toMatch(/^mealquest-/) }) + + it('are v2, so activate deletes the v1 caches', () => { + // v1 API responses held dinners with the pasted recipe_url, which can + // carry a personal token; offline, the worker would still serve them. + expect(cacheNames.api).toBe('mealquest-api-v2') + const current = ['mealquest-precache-v2', ...Object.values(cacheNames)] + const v1 = ['mealquest-precache-v1', 'mealquest-api-v1', 'mealquest-media-v1', 'mealquest-fonts-v1'] + expect(staleCaches([...v1, ...current], current)).toEqual(v1) + }) }) describe('staleCaches', () => { it('deletes only MealQuest caches that are not current', () => { - const current = ['mealquest-precache-v1', ...Object.values(cacheNames)] + const current = ['mealquest-precache-v2', ...Object.values(cacheNames)] const names = [ ...current, 'mealquest-precache', diff --git a/web/src/sw/routes.ts b/web/src/sw/routes.ts index 895fd40..f865452 100644 --- a/web/src/sw/routes.ts +++ b/web/src/sw/routes.ts @@ -25,7 +25,9 @@ export interface RouteRequest { } // Bumping the version renames every cache; activate then deletes the old ones. -export const cacheVersion = 'v1' +// v2 drops v1 API responses, whose dinners still carried the pasted +// recipe_url. +export const cacheVersion = 'v2' export const cachePrefix = 'mealquest-' export const cacheNames = { diff --git a/web/src/sw/worker.test.ts b/web/src/sw/worker.test.ts index 215b9cb..4f223c4 100644 --- a/web/src/sw/worker.test.ts +++ b/web/src/sw/worker.test.ts @@ -1,7 +1,8 @@ // The service worker itself needs worker globals, so these tests read its // source: they pin the parts routes.ts cannot express — the API cache's // strategy, the font strategy's expiration, the font route's place ahead of -// the precache route and network-only requests left to the browser. +// the precache route, network-only requests left to the browser and stale +// caches deleted on activate. import { describe, expect, it } from 'vitest' import source from '../sw.ts?raw' @@ -37,6 +38,12 @@ describe('service worker source', () => { expect(source).not.toContain('registerRoute(() => true') }) + it('deletes every stale cache on activate', () => { + expect(source).toMatch( + /self\.addEventListener\('activate', \(event\) => \{\s*const current = \[workboxCacheNames\.precache, \.\.\.Object\.values\(cacheNames\)\][\s\S]*?staleCaches\(names, current\)\.map\(\(name\) => caches\.delete\(name\)\)/, + ) + }) + it('activates a waiting worker when the update prompt sends SKIP_WAITING', () => { expect(source).toMatch( /self\.addEventListener\('message', \(event\) => \{\s*if \(event\.data\?\.type === 'SKIP_WAITING'\) void self\.skipWaiting\(\)/,