feat(share): read-only share links for decks and binders (#80) - #351
Merged
Conversation
A public `/share/<token>` URL showing one deck or binder and nothing else. scryme is otherwise entirely private, so this is the only route that serves someone who isn't the owner, and it's written that way throughout: nothing is inferred from the visitor's request (no cookies, no preferences, no price source), so two people opening one link see the same page and a visitor's own browser state can't widen what they're shown. #80 left two questions open. Both are answered here, with reasons: * **Prices and owned counts are off by default**, enabled per link. Sending someone a decklist shouldn't also tell them what it's worth or how deep your collection runs — that's a separate thing to volunteer, not a side effect of sharing a list. * **Revocation, not timed expiry.** A clock-based expiry needs a scheduler and a story for what a half-expired link does, and buys little over a Revoke button that acts instantly on the page you shared from. Links show when they were last opened, so a stale one is easy to spot and retire. Tokens are stored hashed, reusing the device-token hashing from #204. A share token is the *most* exposed credential in the system — it travels in a URL people paste into chats and screenshots — so a dump or backup must not hand over working links on top of whatever leaked. A link names one kind and one id rather than a query, so a shared deck can't be widened into the rest of the collection. Unknown, revoked and deleted targets all 404 identically: distinguishing them would confirm to a stranger that a token was once real. Deleting a deck or binder withdraws its links, because one outliving its target would 404 and read as a broken app rather than as "that isn't shared any more". **A test caught a real leak.** The share views originally extended `base.html`, which includes the settings gear and the upload drop zone — so a stranger with a share URL got a link to `/settings`, a preferences panel that PATCHes `/prefs`, and a drop target that posts to `/upload`. Not merely a navigation leak: two write vectors into the owner's instance. Fixed by giving public views their own `share_base.html`, sharing only an extracted `_head.html` so theming can't drift. "Cannot render an owner control" is a stronger guarantee than "is told not to", and the test now names `/prefs`, `/upload`, `/admin` and `/backup` explicitly rather than just checking for nav links. Also: `shared` is declared after `session` on the deck/binder view routes — several tests call those functions directly with positional arguments, and inserting a parameter ahead of `session` silently rebound it. Migration 0035. 24 new tests; suite 1330 passing at 100% coverage. Verified live: a shared 100-card deck renders with zero occurrences of `/prefs`, `/upload`, `/settings`, `hx-post` or `<form>`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LxrePqFWyzQTcWaroSk4sH
| import sqlalchemy as sa | ||
| from alembic import op | ||
|
|
||
| revision: str = "0035_share_link" |
| from alembic import op | ||
|
|
||
| revision: str = "0035_share_link" | ||
| down_revision: str | None = "0034_stack_version" |
…of <head>
The Sonar gate failed with `Web:PageWithoutTitleCheck` on both `base.html` and `share_base.html` —
a real consequence of extracting the shared `<head>`, because SonarHTML can't follow a Jinja include
and so read both shells as pages with no title.
Suppressing the rule would have been the cheap fix, but the right split is different from the one I
made: the `<title>` is genuinely per-page (it's a `{% block %}`), so it belongs in the shell. Only
the invariant parts — meta, fonts, styles, theme hydration, the htmx/alpine bundles — need to be
shared, and those are what actually must not drift between the owner and public shells.
So each shell now owns `<head>`, a literal `<title>`, and an include of `_head_meta.html` for the
rest. Sonar sees a real title because there is one.
Verified live: `/` renders `scryme`, a deck page `<deck> · scryme`, and its share view
`<deck> · shared · scryme`. 1330 passing at 100% coverage.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LxrePqFWyzQTcWaroSk4sH
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #80. A public
/share/<token>URL that shows one deck or binder and nothing else.Written for a stranger
These are the only routes in scryme that serve someone who isn't the owner, so the whole thing is built on that assumption:
Deleting a deck or binder withdraws its links — one outliving its target would 404 and read as a broken app rather than as "that isn't shared any more".
The two open questions, answered
A test caught a real leak
I'd written the argument that share views should use purpose-built templates — then extended
base.htmlanyway. That shell includes the settings gear and the upload drop zone, so a stranger holding a share URL got:/settings,/prefs,/upload.Two write vectors into the owner's instance, not just a navigation leak. Fixed by giving public views their own
share_base.html, sharing only an extracted_head.htmlso theming can't drift between the two. Cannot render an owner control is a stronger guarantee than is told not to — and the test now names/prefs,/upload,/adminand/backupexplicitly rather than only checking for nav links.Verification
ruffandmkdocs --strictclean. Migration 0035 applies and downgrades cleanly./prefs,/upload,/settings,hx-postor<form>. The logo isn't even a link home. Dev share links deleted afterwards.sharedis declared aftersessionon the deck/binder view routes — several tests call those functions directly with positional arguments, and inserting a parameter ahead ofsessionsilently rebound it.🤖 Generated with Claude Code
https://claude.ai/code/session_01LxrePqFWyzQTcWaroSk4sH