Claude: changes for issue #93 #94

Merged

Grandiras merged 1 commit from claude/issue-93 into main

2026-08-18 01:21:13 +00:00

claude-bot commented

2026-08-18 01:16:40 +00:00

Collaborator

Closes #93.

claude-bot added 1 commit

2026-08-18 01:16:41 +00:00

Share cards and boards, and unfurl the links back into cards

CI / build (pull_request) Successful in 31s

Details

a887f53c8e

Anything on the board can now be handed to somebody. ShareLink is the one
place that says what a shareable thing's link is and the one place that
reads one back; a card's link is its board's plus ?card={id}, which the
board strips again the moment it has opened the dialog.

The share itself is a ladder — the platform's sheet, then the clipboard,
then an off-screen selection — resolved against location.origin in the
browser rather than built absolutely on the server, since the browser is
the only party that knows for certain which origin a page is read from.
calendar-interop now re-exports that clipboard fallback instead of
keeping its own.

A shared link survives the sign-in screen: ReturnUrl is the single rule
for whether a value may be a redirect target, applied by the login page
and by /login/start (including on every one of its error redirects) and
honoured by /dev/login so the suite can walk it without OAuth.

A link pasted back into a description or a comment, alone on its line,
renders as the card or board it points at. LinkPreviewService is the one
card-reading path that access-checks, because it is the only one whose id
comes from text a member typed; a card that is missing or not theirs falls
back to the plain link, and the two are deliberately the same answer.

Open Graph tags describe the app and never the page — an unfurler is an
anonymous fetch by somebody else's server, and this board is private.

Adds ShareLinkTests, RichTextTests, ReturnUrlTests and share.spec.js, and
a 2.5.0 changelog entry.

claude-bot referenced this pull request

2026-08-18 01:16:41 +00:00

Implement a share API #93

claude-bot commented

2026-08-18 01:16:43 +00:00

Author

Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/260); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-94/run-260.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-08-18 01:20:51 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

This is a large, well-scoped feature (link sharing, deep-linked cards, link-card unfurling, safe return-URL handling for sign-in, and app-level Open Graph metadata) and it holds up well under review.

Things specifically checked and found correct:

ShareLink.TryParse/SameOrigin: absolute URLs are only accepted when scheme+host+port match the caller-supplied origin, so https://evil.example/board/3?card=9 and lookalike hosts (ponente.app.evil.example, wrong port) are correctly refused — verified against ShareLinkTests. The rooted-path check runs before Uri.TryCreate(..., UriKind.Absolute, ...), which is required on Unix where a bare /board/3 would otherwise parse as a file:// URI (the code comment calls this out explicitly and gets the ordering right).
ReturnUrl.Sanitize: blocks //host, /host, non-local paths, and now also strips any candidate containing control characters (defense against header/redirect injection), applied consistently at /login/start, /login (render), /dev/login/{did}, and Routes.razor's NotAuthorized branch — a single source of truth as the PR claims.
LinkPreviewService: correctly re-checks board visibility per request via BoardService.CanViewAsync/ResolveAsync (same BoardAccess.VisibleTo predicate used everywhere else), and treats missing/denied identically without leaking which case occurred — this is the one new path that reads a Card outside of an already-access-checked circuit, and it's handled the way the architecture notes prescribe.
XSS: LinkCard.razor renders preview.Title/tag names as plain text (not MarkupString), so Blazor auto-encodes them; the only MarkupString use is the existing sanitized Markdown.ToHtml pipeline inside RichTextView, unchanged in that regard.
CSS/markup restructuring (comment-body/composer-preview losing the markdown class on the outer div in favor of a nested one from RichTextView) doesn't break existing styling — verified no CSS rule targets .comment-body/.composer-preview combined with .markdown; the outer divs are pure layout containers and .markdown rules apply the same as before.
og:image points at /icon-512.png, which exists and matches the manifest exactly, consistent with the "full mark, not favicon.svg" rule.
New translation keys (share.*, meta.description) are present and identical in both en/de, satisfying the locale parity test; the changelog entry avoids the jargon guard's banned terms.
LinkPreviewService/BoardService are both scoped, matching DI conventions in this codebase.

Nothing here looks like a blocking bug. A couple of very minor, non-blocking notes: RichTextView's ReferenceEquals memoization guard will rarely hit given fresh decrypted strings on every reload (so it's effectively a no-op optimization in the common case), and share.cards/pluralization isn't handled for the 1 card case — neither is worth blocking on.

⏱ 3m 53s · 22 turns · tokens: 120.0k in / 20.1k out (+2768.2k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This is a large, well-scoped feature (link sharing, deep-linked cards, link-card unfurling, safe return-URL handling for sign-in, and app-level Open Graph metadata) and it holds up well under review. Things specifically checked and found correct: - **`ShareLink.TryParse`/`SameOrigin`**: absolute URLs are only accepted when scheme+host+port match the caller-supplied origin, so `https://evil.example/board/3?card=9` and lookalike hosts (`ponente.app.evil.example`, wrong port) are correctly refused — verified against `ShareLinkTests`. The rooted-path check runs before `Uri.TryCreate(..., UriKind.Absolute, ...)`, which is required on Unix where a bare `/board/3` would otherwise parse as a `file://` URI (the code comment calls this out explicitly and gets the ordering right). - **`ReturnUrl.Sanitize`**: blocks `//host`, `/host`, non-local paths, and now also strips any candidate containing control characters (defense against header/redirect injection), applied consistently at `/login/start`, `/login` (render), `/dev/login/{did}`, and `Routes.razor`'s `NotAuthorized` branch — a single source of truth as the PR claims. - **`LinkPreviewService`**: correctly re-checks board visibility per request via `BoardService.CanViewAsync`/`ResolveAsync` (same `BoardAccess.VisibleTo` predicate used everywhere else), and treats missing/denied identically without leaking which case occurred — this is the one new path that reads a `Card` outside of an already-access-checked circuit, and it's handled the way the architecture notes prescribe. - **XSS**: `LinkCard.razor` renders `preview.Title`/tag names as plain text (not `MarkupString`), so Blazor auto-encodes them; the only `MarkupString` use is the existing sanitized `Markdown.ToHtml` pipeline inside `RichTextView`, unchanged in that regard. - CSS/markup restructuring (`comment-body`/`composer-preview` losing the `markdown` class on the outer div in favor of a nested one from `RichTextView`) doesn't break existing styling — verified no CSS rule targets `.comment-body`/`.composer-preview` combined with `.markdown`; the outer divs are pure layout containers and `.markdown` rules apply the same as before. - `og:image` points at `/icon-512.png`, which exists and matches the manifest exactly, consistent with the "full mark, not favicon.svg" rule. - New translation keys (`share.*`, `meta.description`) are present and identical in both `en`/`de`, satisfying the locale parity test; the changelog entry avoids the jargon guard's banned terms. - `LinkPreviewService`/`BoardService` are both scoped, matching DI conventions in this codebase. Nothing here looks like a blocking bug. A couple of very minor, non-blocking notes: `RichTextView`'s `ReferenceEquals` memoization guard will rarely hit given fresh decrypted strings on every reload (so it's effectively a no-op optimization in the common case), and `share.cards`/pluralization isn't handled for the `1 card` case — neither is worth blocking on. --- ⏱ 3m 53s · 22 turns · tokens: 120.0k in / 20.1k out (+2768.2k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-94/run-260.md)