From 4c09a22addde7ca281105c543358263e1c09583c Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 4 Nov 2026 19:10:09 +0100 Subject: [PATCH] feat(share): a link's address says what it opens A video stays on /watch/, anything else is given out on /file/ and a folder on /folder/. The token is the whole key, so every path opens every link and the links already sent keep working. The new paths are kept out of search indexes like /watch/, boot the player alone, and the unfurl card calls a file a file. --- CLAUDE.md | 2 +- backend/app/downloads/links.py | 18 ++- backend/app/downloads/og.py | 17 ++- backend/app/main.py | 14 ++- backend/app/routes/downloads.py | 10 +- backend/tests/test_share_paths.py | 137 ++++++++++++++++++++++ frontend/public/robots.txt | 6 +- frontend/src/lib/api/downloads.ts | 2 +- frontend/src/watch/WatchPage.dom.test.tsx | 14 +++ frontend/src/watch/WatchPage.tsx | 10 +- frontend/src/watchEntry.contract.test.ts | 23 ++++ frontend/vite.config.ts | 6 +- 12 files changed, 233 insertions(+), 26 deletions(-) create mode 100644 backend/tests/test_share_paths.py diff --git a/CLAUDE.md b/CLAUDE.md index d81043fe..4b6b62a3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,7 +74,7 @@ only along a declared edge and writes them only through that unit's service; cor cannot (an ORM write on another unit's loaded row, SQL in `text()`, a commit by `begin()`) is review's to catch, listed in [`backend/app/README.md`](backend/app/README.md) with the conventions. Frontend, by folder under `src/`: `main.tsx` provider tree → `shell/` (`App`, header, nav rail, palette, -boot; a shared `/watch` link boots from its own entry, `watch.html` → `watchMain.tsx` → `watch/`, held +boot; a share link (`/watch`, `/file`, `/folder`) boots from its own entry, `watch.html` → `watchMain.tsx` → `watch/`, held app-free by a contract test) → **`modules//`** (one folder per page: the page, its rail, its store, its tests, and its `module.ts` definition — label, icon, gate, the lazy page and rail, its store — collected in nav order by `features/navigation/moduleRegistry`, which also mounts every store at the root; a new page is its folder, its id in diff --git a/backend/app/downloads/links.py b/backend/app/downloads/links.py index 54546761..68e5b6c5 100644 --- a/backend/app/downloads/links.py +++ b/backend/app/downloads/links.py @@ -127,14 +127,28 @@ def check_grant(token: str, grant: str | None, password_version: int) -> bool: # --- serialization ------------------------------------------------------------------------- -def owner_view(link: DownloadLink, base_url: str = "", token: str | None = None) -> dict: +def share_path(db: Session, link: DownloadLink, token: str) -> str: + """The address a link is given out on: its path says what it opens. Only a word for the reader, + the token is the whole key: every prefix of `main.SHARE_PREFIXES` opens every link, so the links + already sent on /watch/ work for good. A file still being made reads as the video it mostly is.""" + from app.downloads import mediakind + + if link.folder_id is not None: + return f"/folder/{token}" + job = db.get(DownloadJob, link.job_id) + asset = db.get(MediaAsset, job.asset_id) if job and job.asset_id else None + kind = asset.media_kind if asset else None + return f"/file/{token}" if kind and kind != mediakind.VIDEO else f"/watch/{token}" + + +def owner_view(db: Session, link: DownloadLink, token: str | None = None) -> dict: """What the file owner sees when managing their links. `token` is the one just made (it is in hand even where no copy is kept); else it is read back, and `url` is None when it cannot be.""" token = token or recover(link) return { "id": link.id, "token": token, - "url": f"{base_url}/watch/{token}" if token else None, + "url": share_path(db, link, token) if token else None, "allow_download": link.allow_download, "has_password": link.password_hash is not None, "expires_at": link.expires_at.isoformat() if link.expires_at else None, diff --git a/backend/app/downloads/og.py b/backend/app/downloads/og.py index f2d4d29c..626e4e1c 100644 --- a/backend/app/downloads/og.py +++ b/backend/app/downloads/og.py @@ -1,4 +1,4 @@ -"""Server-rendered Open Graph tags for the public /watch/{token} page. +"""Server-rendered Open Graph tags for the public page of a share link (/watch/, /file/, /folder/). The watch page is a client-side SPA, so a social crawler (facebookexternalhit, Twitterbot, …) that fetches /watch/{token} would otherwise only see the generic index.html — a blank/greyed link card. @@ -76,16 +76,21 @@ def _watch_tags(token: str, db: Session) -> str | None: link = linksmod.find(db, token) if link is None or linksmod.is_expired(link): return None - url = f"{settings.app_base}/watch/{token}" + url = f"{settings.app_base}{linksmod.share_path(db, link, token)}" folder = link.folder_id is not None + # What the card calls it, as its address does (`links.share_path`). + video = "/watch/" in url if link.password_hash: # Password-gated: don't leak the title/thumbnail in the unfurl. return "\n ".join( [ "Siftlode", - _tag("og:title", "Password-protected " + ("folder" if folder else "video")), + _tag( + "og:title", + "Password-protected " + ("folder" if folder else "video" if video else "file"), + ), _tag("og:description", "Shared via Siftlode"), - _tag("og:type", "website" if folder else "video.other"), + _tag("og:type", "video.other" if video else "website"), _tag("og:url", url), _tag("og:site_name", "Siftlode"), ] @@ -96,7 +101,7 @@ def _watch_tags(token: str, db: Session) -> str | None: asset = db.get(MediaAsset, job.asset_id) if job and job.asset_id else None if job is None or asset is None or asset.status != "ready": return None - title = (job.display_name or asset.title or "Video").strip() + title = (job.display_name or asset.title or ("Video" if video else "File")).strip() channel = (job.display_uploader or asset.uploader or "").strip() # Pick the link-preview image. Facebook's scraper REJECTS our own poster ("Corrupted Image" in the # Sharing Debugger): the poster is an ffmpeg-produced JPEG whose non-standard marker order FB's @@ -118,7 +123,7 @@ def _watch_tags(token: str, db: Session) -> str | None: f"{html.escape(title)}", _tag("og:title", title), _tag("og:description", channel or "Shared via Siftlode"), - _tag("og:type", "video.other"), + _tag("og:type", "video.other" if video else "website"), _tag("og:url", url), _tag("og:site_name", "Siftlode"), _tag("twitter:title", title, attr="name"), diff --git a/backend/app/main.py b/backend/app/main.py index 30905c8d..1f1702a4 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -368,6 +368,10 @@ INDEX_HTML = STATIC_DIR / "index.html" # A public share link's own page: the player alone, without the app's bundle (frontend/watch.html). WATCH_HTML = STATIC_DIR / "watch.html" INDEX_HEADERS = {"Cache-Control": "no-cache"} +# The paths a share link opens on, by what it opens (`links.share_path`). Any of them opens any link. +# A new one also goes in frontend/public/robots.txt, the Vite dev entry and WatchPage's token parsing +# (watchEntry.contract.test.ts). +SHARE_PREFIXES = ("watch/", "file/", "folder/") app.mount( "/assets", StaticFiles(directory=STATIC_DIR / "assets", check_dir=False), @@ -381,8 +385,10 @@ async def index() -> FileResponse: @app.get("/watch/{token}") -async def watch_page(token: str): - """Serve the public player page for a share link, with per-video Open Graph tags injected so the +@app.get("/file/{token}") +@app.get("/folder/{token}") +async def share_page(token: str): + """Serve the public page of a share link, with per-link Open Graph tags injected so the link unfurls richly (title/channel/thumbnail) in Messenger/social crawlers. A real browser ignores the extra tags and boots WatchPage; invalid/expired/password links fall back to the generic card.""" @@ -409,8 +415,8 @@ async def spa_fallback(full_path: str) -> FileResponse: # Real SPA-root assets (welcome images, favicon, robots.txt) rarely change and have # stable names, so a moderate cache is safe and satisfies the cache-lifetime audit. return FileResponse(candidate, headers={"Cache-Control": "public, max-age=604800"}) - # A /watch/ path the route above does not match (a trailing segment, no token) is still the + # A share path the route above does not match (a trailing segment, no token) is still the # player's: WatchPage reads the token off the path and says when a link is gone. - if full_path.startswith("watch/"): + if full_path.startswith(SHARE_PREFIXES): return FileResponse(WATCH_HTML, headers=INDEX_HEADERS) return FileResponse(INDEX_HTML, headers=INDEX_HEADERS) diff --git a/backend/app/routes/downloads.py b/backend/app/routes/downloads.py index 380bb337..4730f9d0 100644 --- a/backend/app/routes/downloads.py +++ b/backend/app/routes/downloads.py @@ -611,7 +611,7 @@ def list_links( job_id: int, user: User = Depends(current_user), db: Session = Depends(get_db) ) -> list[dict]: own_job(db, user, job_id) - return [linksmod.owner_view(link) for link in queries.links_to(db, job_id=job_id)] + return [linksmod.owner_view(db, link) for link in queries.links_to(db, job_id=job_id)] class CreateLinkIn(BaseModel): @@ -653,7 +653,7 @@ def create_link( raise refused(409, "share_unfinished") link, token = _new_link(db, user, body, job_id=job_id) db.commit() - return linksmod.owner_view(link, token=token) + return linksmod.owner_view(db, link, token=token) def _shareable_job(db: Session, user: User, job_id: int) -> "DownloadJob": @@ -684,7 +684,7 @@ def list_folder_links( folder_id: int, user: User = Depends(current_user), db: Session = Depends(get_db) ) -> list[dict]: own_folder(db, user, folder_id) - return [linksmod.owner_view(link) for link in queries.links_to(db, folder_id=folder_id)] + return [linksmod.owner_view(db, link) for link in queries.links_to(db, folder_id=folder_id)] @router.post("/folders/{folder_id}/links") @@ -697,7 +697,7 @@ def create_folder_link( _shareable_folder(db, user, folder_id) link, token = _new_link(db, user, body, folder_id=folder_id) db.commit() - return linksmod.owner_view(link, token=token) + return linksmod.owner_view(db, link, token=token) class UpdateLinkIn(BaseModel): @@ -724,7 +724,7 @@ def update_link( sent=frozenset(body.model_fields_set), ) db.commit() - return linksmod.owner_view(link) + return linksmod.owner_view(db, link) @router.delete("/links/{link_id}") diff --git a/backend/tests/test_share_paths.py b/backend/tests/test_share_paths.py new file mode 100644 index 00000000..3469a1df --- /dev/null +++ b/backend/tests/test_share_paths.py @@ -0,0 +1,137 @@ +"""A share link's address says what it opens: /watch/ a video, /file/ anything else, /folder/ a folder. + +Only a word for the reader: the token is the key, so every path opens every link, and the links +already sent out on /watch/ keep working for good. DB-lane. +""" + +import pytest +from cryptography.fernet import Fernet + +from app import main, security +from app.config import settings +from app.downloads import og +from app.downloads import probe as probemod + +_PAGE = """ + +""" + +_VIDEO = { + "streams": [ + {"codec_type": "video", "codec_name": "h264", "pix_fmt": "yuv420p"}, + {"codec_type": "audio", "codec_name": "aac"}, + ], + "format": {"format_name": "mov,mp4", "duration": "5"}, +} +_AUDIO = { + "streams": [{"codec_type": "audio", "codec_name": "aac"}], + "format": {"format_name": "mov,mp4", "duration": "5"}, +} + + +@pytest.fixture +def pages(tmp_path, monkeypatch): + index, watch = tmp_path / "index.html", tmp_path / "watch.html" + index.write_text(_PAGE.format(entry="app"), encoding="utf-8") + watch.write_text(_PAGE.format(entry="watch"), encoding="utf-8") + monkeypatch.setattr(main, "INDEX_HTML", index) + monkeypatch.setattr(main, "WATCH_HTML", watch) + + +@pytest.fixture +def key(monkeypatch): + """A link's address is read back from its encrypted token.""" + monkeypatch.setattr(security, "_fernet", Fernet(Fernet.generate_key())) + + +@pytest.fixture +def shared(as_user, make_user, tmp_path, monkeypatch, upload_headers): + """Uploads a file ffprobe reads as `data` (None: it cannot read it) and shares it.""" + monkeypatch.setattr(settings, "files_root", str(tmp_path / "media")) + owner = as_user(make_user()) + + def share(name: str, data: dict | None, **link: object) -> dict: + monkeypatch.setattr(probemod, "probe_json", lambda _p: data) + job = owner.post( + "/api/files/upload", + content=b"some bytes", + headers=upload_headers(name, display_name=name), + ) + assert job.status_code == 200, job.text + made = owner.post(f"/api/downloads/{job.json()['id']}/links", json=link) + assert made.status_code == 200, made.text + return made.json() + + share.owner = owner # type: ignore[attr-defined] + return share + + +@pytest.mark.parametrize( + ("name", "data", "prefix"), + [ + ("clip.mp4", _VIDEO, "/watch/"), + ("song.m4a", _AUDIO, "/file/"), + ("backup.tar.gz", None, "/file/"), + ], +) +def test_a_link_is_given_out_on_the_path_of_what_it_opens(shared, name, data, prefix): + made = shared(name, data) + assert made["url"] == f"{prefix}{made['token']}" + + +def test_a_folder_link_is_given_out_on_folder(shared, key): + owner = shared.owner + folder = owner.post("/api/files/folders", json={"name": "Friends"}).json() + made = owner.post(f"/api/downloads/folders/{folder['id']}/links", json={}).json() + assert made["url"] == f"/folder/{made['token']}" + listed = owner.get(f"/api/downloads/folders/{folder['id']}/links").json() + assert [entry["url"] for entry in listed] == [made["url"]] + + +def test_the_list_gives_the_same_address_as_the_making(shared, key): + made = shared("backup.tar.gz", None) + owner = shared.owner + files = owner.get("/api/files").json() + job_id = next(f["id"] for f in files if f["display_name"] == "backup.tar.gz") + listed = owner.get(f"/api/downloads/{job_id}/links").json() + assert [entry["url"] for entry in listed] == [made["url"]] + + +@pytest.mark.parametrize("path", ["watch", "file", "folder"]) +@pytest.mark.parametrize( + ("name", "data"), [("clip.mp4", _VIDEO), ("backup.tar.gz", None)], ids=["video", "file"] +) +def test_every_path_opens_every_link(pages, shared, client, path, name, data): + """The links already sent on /watch/ open whatever they open, for good.""" + token = shared(name, data)["token"] + r = client.get(f"/{path}/{token}") + assert r.status_code == 200 + assert "/assets/watch.js" in r.text and "/assets/app.js" not in r.text + assert f'content="{name}"' in r.text + + +@pytest.mark.parametrize("path", ["/file/no-such-token", "/folder/no-such-token", "/file/a/b"]) +def test_a_new_path_without_a_live_link_still_gets_the_player(pages, client, path): + r = client.get(path) + assert r.status_code == 200 + assert "/assets/watch.js" in r.text and "/assets/app.js" not in r.text + + +def test_a_card_names_a_file_as_a_file(shared, db): + made = shared("backup.tar.gz", None) + tags = og._watch_tags(made["token"], db) + assert f'{made["url"]}"' in tags + assert "video.other" not in tags + + locked = shared("notes.tar.gz", None, password="pw") + hidden = og._watch_tags(locked["token"], db) + assert "Password-protected file" in hidden and "notes" not in hidden + assert "video.other" not in hidden + + +def test_a_card_of_a_video_stays_a_video(shared, db): + made = shared("clip.mp4", _VIDEO) + tags = og._watch_tags(made["token"], db) + assert f"/watch/{made['token']}" in tags and "video.other" in tags + locked = shared("other.mp4", _VIDEO, password="pw") + assert "Password-protected video" in og._watch_tags(locked["token"], db) diff --git a/frontend/public/robots.txt b/frontend/public/robots.txt index 5d456ccc..41b1e8d3 100644 --- a/frontend/public/robots.txt +++ b/frontend/public/robots.txt @@ -1,14 +1,16 @@ # Siftlode is mostly a private, login-gated app; only the public landing and legal # pages are worth indexing. Allow crawling but keep bots out of the API surface. -# Share links (/watch/) are unguessable capability URLs: kept out of search indexes, +# Share links (/watch/, /file/, /folder/) are unguessable capability URLs: kept out of search indexes, # but link-preview crawlers are allowed below so shared links unfurl in chat apps. User-agent: * Allow: / Disallow: /api/ Disallow: /auth/ Disallow: /watch/ +Disallow: /file/ +Disallow: /folder/ -# Link-preview / unfurl bots — allowed to fetch /watch/ so shared links render a rich +# Link-preview / unfurl bots — allowed to fetch share links so shared links render a rich # card (title/channel/thumbnail). They don't build a search index, so this doesn't # expose share links to search engines. User-agent: facebookexternalhit diff --git a/frontend/src/lib/api/downloads.ts b/frontend/src/lib/api/downloads.ts index acc06f26..93d4ee68 100644 --- a/frontend/src/lib/api/downloads.ts +++ b/frontend/src/lib/api/downloads.ts @@ -145,7 +145,7 @@ export interface ShareLink { // Null when the server cannot show this link's address again (no encrypted copy its key reads): the // link still opens for whoever has it; to share it anew, make a new link. token: string | null; - url: string | null; // path like "/watch/" — prepend window.location.origin to share + url: string | null; // path like "/watch/" or "/file/" — prepend window.location.origin to share allow_download: boolean; has_password: boolean; expires_at: string | null; diff --git a/frontend/src/watch/WatchPage.dom.test.tsx b/frontend/src/watch/WatchPage.dom.test.tsx index ff6883bc..3a47d563 100644 --- a/frontend/src/watch/WatchPage.dom.test.tsx +++ b/frontend/src/watch/WatchPage.dom.test.tsx @@ -24,6 +24,20 @@ afterEach(() => { localStorage.clear(); }); +describe("WatchPage share paths", () => { + it.each(["/watch/tok-p", "/file/tok-p", "/folder/tok-p", "/file/tok-p/extra"])( + "reads the token off %s", + async (path) => { + window.history.replaceState({}, "", path); + mockMeta({ needs_password: false, audio: false, file_url: "/f.mp4", title: "A clip" }); + render(); + await waitFor(() => + expect(fetch).toHaveBeenCalledWith("/api/public/watch/tok-p", expect.anything()), + ); + }, + ); +}); + describe("WatchPage background play", () => { it("hands its