Repository navigation
Links in text are clickable, and say what is at the other end - #37
Merged
Merged
Conversation
A URL in any of a board's text is a link, and hovering one for a second opens a panel with the page's title, description, thumbnail and real host — or says it could not be reached. Nothing marks a link up: the store holds a plain string, so `core/links.js` is a recogniser that runs on the way out and the document keeps the characters somebody typed. An explicit scheme or `www.` counts; a bare domain does not, because `readme.md` and `3.14` would otherwise turn blue. Trailing punctuation is prose, and a closing bracket the URL did not open is too — which is the difference between a Wikipedia link and a sentence about one. **The preview cannot be fetched by the browser.** A cross-origin `fetch` in `cors` mode rejects whatever the server said; `no-cors` resolves opaque, status 0, body unreadable, a 404 identical to a 200. So "2xx, and here is its title" is not something one origin may learn about another, and `supabase/functions/link- preview` does the fetch — plain JavaScript, so its guard and its extraction are imported by `node --test` rather than needing a runtime. Previews therefore join sharing and live edits as things a project provides; without one, a hover says so rather than blaming the page. Nothing that comes back is executed, five ways: the panel is built from nodes and not markup, the scheme is allow-listed at render *and* at open, the target page is never rendered, the function evaluates nothing it fetched, and anchors carry `rel="noopener noreferrer"`. The function is not an open proxy or an SSRF hole: `verify_jwt` is on, and `guard.js` refuses every scheme but http/https, credentials in a URL, every address that is not on the internet in both families, names only a private network resolves, whatever a name resolves *to*, and each redirect separately. A refusal answers exactly what a silence answers, so it cannot be used to scan the network it runs inside. The popover is drawn imperatively in the overlay with the cursors rather than in React, which is the line this app already draws between chrome that shows state and chrome that follows a pointer — and is what lets it be driven with a hand-run clock and a stubbed fetcher. Two things found by writing the tests: a hover that dispatched no `pointerover` because CDP leaves the pointer where the last test put it, which had two tests passing against the *app's* popover instead of the one under test; and a press on a link really opening a tab in Chromium, which left every later input dispatch five seconds slow. 856 tests, 96.4% lines. Every new module at 100%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DGAkrDAKvwwHUFceiFYinQ
`preview` takes its `fetch` as an argument now, for the reason everything in `src/` takes its environment as one — redirect following, the byte caps, the thumbnail and the not-a-page branch were the parts most worth pinning and the parts a unit test of the guard cannot reach. The default is still the real `fetch`, and `guard.js` runs on every URL either way: the seam decides who answers, never what is allowed. `Deno.serve` is now called only where there is a Deno, so importing the module does not start a server. Twenty-two tests came with it, and one bug: `typeof Deno?.resolveDns` still evaluates `Deno`, and an identifier that was never declared is a ReferenceError rather than undefined — so the resolution guard threw everywhere outside the edge runtime. It would not have fired in production, and it would have hidden in a module nothing could import. The two that matter most are the SSRF pair the guard alone cannot show: a public URL redirecting to 169.254.169.254 is not followed, and an og:image pointing there is not fetched — both asserted on what the fake fetch was *asked*, not only on what came back. 878 tests, 96.4% lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DGAkrDAKvwwHUFceiFYinQ
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change adds clickable HTTP(S) and ChangesBoard links and previews
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant BoardCanvas
participant createApp
participant createLinks
participant linkPreview
participant link_preview_Edge_Function
BoardCanvas->>createApp: configure fetchPreview
createApp->>createLinks: create hover controller
createLinks->>linkPreview: request preview URL
linkPreview->>link_preview_Edge_Function: invoke authenticated function
link_preview_Edge_Function-->>linkPreview: return preview payload
linkPreview-->>createLinks: normalized preview
createLinks-->>BoardCanvas: render overlay
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
supabase/functions/link-preview/guard.js (1)
53-67: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
blockedV4does not validate the fourth octet.The signature destructures only
[a, b, c], so1.2.3.999passes the "not an address at all" check. The WHATWG URL parser rejects that host beforecheckUrlreaches here, so the gap is not reachable throughcheckUrltoday.isBlockedAddressis exported, so the guard is still worth completing.🛡️ Proposed fix
-function blockedV4([a, b, c]) { - if ([a, b, c].some((part) => part > 255)) return true; // not an address at all +function blockedV4([a, b, c, d]) { + if ([a, b, c, d].some((part) => !Number.isInteger(part) || part > 255)) return true; // not an address at all🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/functions/link-preview/guard.js` around lines 53 - 67, Update blockedV4 to validate all four IPv4 octets, including the fourth octet, before applying the blocked-range checks. Preserve the existing range logic and ensure invalid addresses such as 1.2.3.999 return blocked when reached through the exported isBlockedAddress path.src/core/links.js (1)
142-143: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
hasLinkis not a cheap early out.
hasLinkbuilds the complete run list before it answers. The comment says the opposite. A short-circuit stops at the first followable match.♻️ Proposed short-circuit
-/** Whether text holds anything worth rendering as a link. Cheap early out. */ -export const hasLink = (text) => linkRuns(text).some((run) => Boolean(run.href)); +/** Whether text holds anything worth rendering as a link. Cheap early out. */ +export function hasLink(text) { + for (const match of String(text ?? '').matchAll(LINK)) { + if (hrefFor(match[0])) return true; + } + return false; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/links.js` around lines 142 - 143, Update hasLink in the link-detection flow to stop scanning as soon as the first run with a truthy href is found, rather than building the complete linkRuns result before checking it. Preserve the boolean return behavior for both matching and non-matching text, and revise the misleading “Cheap early out” comment to reflect the implementation.test/browser/links.test.js (1)
422-425: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis assertion does not check what its name claims.
beforeEachcallsapp.links.destroy()at line 118, anddestroy()returns nothing and leaves the object in place.Boolean(app.links)is therefore true for a torn-down layer, and the message "nothing hovers at all" cannot be produced by this test. Assert something the destroyed layer cannot satisfy, or move this test above thebeforeEachteardown.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/browser/links.test.js` around lines 422 - 425, Update the “the app still wires a links layer” test to verify usable link-layer behavior rather than merely checking that app.links is truthy after beforeEach teardown. Assert a property or behavior that app.links.destroy() cannot leave satisfied, or move the test before the teardown so it validates the intended wiring state.supabase/functions/link-preview/index.js (1)
137-151: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider a budget for the whole request, not only per hop.
TIMEOUT_MSapplies to eachfetchImplcall. One preview can make up to four page requests plus four thumbnail requests. The worst case is about eight timesTIMEOUT_MSbefore the function answers, and the caller holds a hover panel on "loading" for all of it. A single deadline shared by the page fetch and the thumbnail fetch bounds it.Also applies to: 228-229
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/functions/link-preview/index.js` around lines 137 - 151, Update fetchGuarded to use a single request-wide deadline shared across all redirect hops, and pass the remaining budget to each fetchImpl AbortSignal instead of resetting TIMEOUT_MS per hop. Propagate that same deadline through the page and thumbnail fetch flows around the additional referenced calls, ensuring the combined operation returns within one bounded timeout.supabase/functions/link-preview/extract.js (1)
50-55: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
\balso matches a hyphenated attribute name.
\bname\s*=matchesdata-name=, and\bcontent\s*=matchesdata-content=, because-is a non-word character. A tag such as<meta data-name="og:title" content="X">is then read with the wrong key. The result is a wrong title rather than a security problem, so the priority is low.The regex is also recompiled for every tag and every key. Precompiled patterns avoid that.
♻️ Proposed change
-const attribute = (tag, name) => { - const match = new RegExp(`\\b${name}\\s*=\\s*("([^"]*)"|'([^']*)'|([^\\s"'>]+))`, 'i').exec(tag); - if (!match) return null; - return match[2] ?? match[3] ?? match[4] ?? null; -}; +const ATTRIBUTES = new Map(['property', 'name', 'content'].map((name) => [ + name, + new RegExp(`(?:^|[\\s/])${name}\\s*=\\s*("([^"]*)"|'([^']*)'|([^\\s"'>]+))`, 'i'), +])); + +const attribute = (tag, name) => { + const match = ATTRIBUTES.get(name)?.exec(tag); + if (!match) return null; + return match[2] ?? match[3] ?? match[4] ?? null; +};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/functions/link-preview/extract.js` around lines 50 - 55, Update the attribute parser around the attribute function so attribute names are matched only at the start of the tag or after a character that cannot be part of an attribute name, preventing data-name from matching name or data-content from matching content. Reuse precompiled patterns for the supported keys instead of constructing a new RegExp on every tag and lookup, while preserving quoted and unquoted value handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 249-252: Update the link preview description in the README to
distinguish pages that return non-2xx responses from pages that are unreachable.
Replace the current “could not be reached” wording with language indicating the
preview is unavailable or the page returned a non-2xx response, consistent with
the status-aware behavior in links.js.
- Around line 254-256: Rewrite the sentence beginning “Nothing marks a link up”
to state explicitly that storage retains plain text, while rendering recognizes
URL-like text as links; keep the surrounding explanation about core/links.js
consistent with this distinction.
- Around line 262-267: The README’s CORS explanation incorrectly says cors-mode
fetch always rejects cross-origin responses. Update the paragraph describing the
edge function and cross-origin fetch so cors mode succeeds when the target
provides a matching Access-Control-Allow-Origin header, while noting arbitrary
pages usually do not, and preserve the explanation that no-cors produces an
unreadable opaque response.
In `@src/platform/input.js`:
- Around line 113-133: Prevent native activation of board links by adding click
handling for [data-link] anchors that cancels the browser’s default navigation
while preserving the existing openLink behavior. Update the relevant input event
flow near startDrag and ensure the test verifies no additional native tab or
navigation occurs when a board link is clicked.
In `@src/platform/links.js`:
- Around line 203-218: Update the open function’s fetchPreview result handling
so a null or undefined answer follows the same failure path as a thrown request:
render “unavailable” only when the panel is still active for the requested href,
then return without calling remember or reading answer.ok. Ensure the scheduler
callback invoking open cannot produce an unhandled rejection for missing
answers.
In `@supabase/functions/link-preview/index.js`:
- Around line 158-167: Cancel response.body before returning when absoluteUrl
cannot resolve the redirect target, matching the existing cancellation performed
for followed redirects. Update the next-is-null branch in the redirect handling
flow while preserving the returned status response.
In `@test/browser/links.test.js`:
- Around line 290-297: Replace the fixed sleeps with readiness waits in both
affected sites: in test/browser/links.test.js:290-297, replace the 80ms sleep
before the assertion with page.waitFor checking window.asked.length === 2 and
label it “the second question”; in test/browser/links.test.js:401-419, replace
the 40ms sleep after the y: 700 patch with page.waitFor for the anchor’s updated
position before reading the box, ensuring it cannot use the pre-move layout.
---
Nitpick comments:
In `@src/core/links.js`:
- Around line 142-143: Update hasLink in the link-detection flow to stop
scanning as soon as the first run with a truthy href is found, rather than
building the complete linkRuns result before checking it. Preserve the boolean
return behavior for both matching and non-matching text, and revise the
misleading “Cheap early out” comment to reflect the implementation.
In `@supabase/functions/link-preview/extract.js`:
- Around line 50-55: Update the attribute parser around the attribute function
so attribute names are matched only at the start of the tag or after a character
that cannot be part of an attribute name, preventing data-name from matching
name or data-content from matching content. Reuse precompiled patterns for the
supported keys instead of constructing a new RegExp on every tag and lookup,
while preserving quoted and unquoted value handling.
In `@supabase/functions/link-preview/guard.js`:
- Around line 53-67: Update blockedV4 to validate all four IPv4 octets,
including the fourth octet, before applying the blocked-range checks. Preserve
the existing range logic and ensure invalid addresses such as 1.2.3.999 return
blocked when reached through the exported isBlockedAddress path.
In `@supabase/functions/link-preview/index.js`:
- Around line 137-151: Update fetchGuarded to use a single request-wide deadline
shared across all redirect hops, and pass the remaining budget to each fetchImpl
AbortSignal instead of resetting TIMEOUT_MS per hop. Propagate that same
deadline through the page and thumbnail fetch flows around the additional
referenced calls, ensuring the combined operation returns within one bounded
timeout.
In `@test/browser/links.test.js`:
- Around line 422-425: Update the “the app still wires a links layer” test to
verify usable link-layer behavior rather than merely checking that app.links is
truthy after beforeEach teardown. Assert a property or behavior that
app.links.destroy() cannot leave satisfied, or move the test before the teardown
so it validates the intended wiring state.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b747a987-015f-4b0e-9e88-ce89efad9e93
📒 Files selected for processing (23)
README.mdsrc/app.jssrc/components/BoardCanvas.jsxsrc/core/bar-position.jssrc/core/links.jssrc/platform/input.jssrc/platform/link-preview.jssrc/platform/links.jssrc/platform/views.jssrc/shell/link-preview.jssrc/styles/canvas.csssupabase/config.tomlsupabase/functions/link-preview/extract.jssupabase/functions/link-preview/guard.jssupabase/functions/link-preview/index.jstest/browser/interact.test.jstest/browser/links.test.jstest/browser/views.test.jstest/node/bar-position.test.jstest/node/link-preview-fetch.test.jstest/node/link-preview-function.test.jstest/node/link-preview.test.jstest/node/links.test.js
…ody owned - **A malformed answer became an unhandled rejection.** `fetchPreview` is injected, so reading `.ok` off whatever it resolves is trusting the thing the boundary exists to distrust: `null` threw inside a promise nobody awaits, leaving the panel on "loading" for ever. A missing answer is now the same case as a thrown one — our failure, not the page's — and the timer's call has a backstop `catch`. - **The anchor's own activation is cancelled now.** Cancelling `pointerdown` does not cancel the `click`, and `target="_blank"` would mean a second tab beside the one we opened. I probed it: `defaultPrevented` was `false`, so nothing of ours owned that gesture and the only reason no tab appeared is that Chrome does not activate links inside a `contenteditable`. That is the browser's habit, not our decision. Still not cancelled while the object is being edited, where the click belongs to the caret. - **The CORS explanation was wrong**, in four places. A `cors` fetch does not "reject whatever the server said" — it succeeds when the target sends an `Access-Control-Allow-Origin` naming the caller, and rejects because an arbitrary page has no reason to. The conclusion is unchanged; the reason given for it was not, and it was repeated in three doc comments and the README. - A redirect body is cancelled before the target is checked rather than after, so the one path that returned early no longer leaves the stream open. - Two fixed sleeps became conditions: the second question being asked, and the link actually reaching the bottom of the window before its box is read. - The README says a non-2xx page reads as unreachable *and that the panel names the status*, since a 404 and a silence are different things to act on; and it states plainly that storage keeps plain text while the view does the recognising. 881 tests, 96.4% lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DGAkrDAKvwwHUFceiFYinQ
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/platform/links.js`:
- Around line 226-229: Update the answer validation guard in the controller to
also require typeof answer.ok === 'boolean' before caching or rendering the
response. Keep malformed objects such as {} on the existing unavailable path so
they remain retryable client-side failures.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 30f4ef23-ab14-45c2-917e-b27e5bd0722a
📒 Files selected for processing (7)
README.mdsrc/platform/input.jssrc/platform/link-preview.jssrc/platform/links.jssupabase/functions/link-preview/index.jstest/browser/interact.test.jstest/browser/links.test.js
🚧 Files skipped from review as they are similar to previous changes (6)
- src/platform/link-preview.js
- test/browser/links.test.js
- README.md
- test/browser/interact.test.js
- src/platform/input.js
- supabase/functions/link-preview/index.js
`{}` is an object, so it passed the guard added in 3da036f, got cached, and drew
"unreachable" — which blames the page for a fetcher that answered nonsense.
`typeof answer.ok === 'boolean'` keeps every malformed shape in the one bucket
where it belongs: our failure, retryable, not remembered.
The test is now three shapes rather than one, and each also asserts the answer
was not cached, since a fetcher that answers nonsense once should be asked again
rather than having its nonsense drawn for the rest of the session.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DGAkrDAKvwwHUFceiFYinQ
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/browser/links.test.js`:
- Around line 320-326: Replace the fixed page.sleep(40) in the retry-hover
sequence with an explicit wait until [data-link-popover] is absent, then perform
hover(box). Keep the existing pointer movement, waitOut, and second-question
assertion unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0e3018cd-81ab-442b-a880-74245126d242
📒 Files selected for processing (2)
src/platform/links.jstest/browser/links.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/platform/links.js
Five places moved the pointer off a link and then slept before hovering again. Under load the next hover could start while the last panel was still up, and the assertion after it would have been about the wrong one — so there is one `leave` helper now that waits for `[data-link-popover]` to be absent, which is a real condition and reads better at every call site. Two sleeps are left, both before absence assertions that have no positive condition of their own — and both after a call that has already settled what they check: `open` renders synchronously before its first `await`, so a wrongly armed timer would have drawn a panel by the time `flushTimers()` returns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DGAkrDAKvwwHUFceiFYinQ
Owner
Author
|
@coderabbitai review |
|
The function is live on the Paris project, and `Deno.resolveDns` is present there — worth checking rather than assuming, because the guard around it means a runtime without it would lose the resolution check silently. `http://10.0.0.1.nip.io/` and `http://localtest.me/` are public names that resolve into private space: both answered `{ ok: false, status: 0 }` and both logged `refused: resolves-private`, so nothing was fetched. A name that resolves nowhere logs no refusal and reaches the same answer through the fetch failing, which is the difference the code intends. DNS rebinding is still open, and still written down as such. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DGAkrDAKvwwHUFceiFYinQ
Owner
Author
Deployed and verified from outside
Checked against the real internet, with a key rather than from the app:
The DNS check does fireThis is the open question from the description, and it is now closed.
Both logged Recorded in the README in b765e27. DNS rebinding remains open and remains written down. One thing to know about the last review roundCodeRabbit reported "Review rate limited" on the 5b8ff0e push, and a rate-limited refusal still shows the check as Nothing calls the function until this merges and Pages redeploys, so it is live and idle until then. |
This was referenced Aug 9, 2026
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.
A URL in any of a board's text is a link, and hovering one for a second says what is at the other end — title, description, thumbnail and the host it really came from, or that it could not be reached.
Links are recognised, not marked up
The store holds a plain string. That is what a paste is forced into and what
innerTextreads back, so nothing marks a link up andcore/links.jsis a recogniser that runs on the way out, in the view. The document keeps the characters somebody typed.An explicit scheme or a
www.counts. A bare domain deliberately does not —readme.md,e.g.somethingand3.14would all turn blue, and a link nobody asked for is worse than one they have to type six characters for. Trailing punctuation is prose, and so is a closing bracket the URL did not open, which is the difference betweenhttps://en.wikipedia.org/wiki/Ruby_(gem)and a sentence that mentions it.Clicking opens a new tab. Dragging an object that happens to hold a link is a drag, shift-clicking is a selection, and inside the field being edited a click belongs to the caret — the same rule that lets you put the cursor anywhere in a card you are writing. A URL typed into a card becomes a link when the field is left, because the view will not write to a focused field and nothing else asks for a redraw: leaving one records history without applying an op.
The preview cannot be fetched by the browser
This is the part worth reading before the code. A cross-origin
fetchincorsmode rejects whatever the server said; inno-corsmode it resolves with an opaque response whose status is0and whose body cannot be read, identically for a 200 and a 404. "Did it answer 2xx, and what is on it" is not a thing one origin may learn about another.So
supabase/functions/link-previewdoes the fetch — this repo's first Edge Function — and the client asks it throughclient.functions.invoke, which carries the session thatverify_jwtchecks. Previews therefore join sharing, live edits and cursors as things a project provides. With none configured, a hovered link says so rather than sitting on "loading" or blaming the page, and there is a test for that state because it is the state the published site is in until this is deployed.It is written in plain JavaScript. There is no TypeScript anywhere else here, and writing it in JS is what lets its two halves that hold all the judgement —
guard.jsandextract.js— be imported by thenode --testsuite that already exists.previewtakes itsfetchas an argument too, so redirect following, the caps, the thumbnail and the not-a-page branch are driven offline as well; the only unreachable line isDeno.serve.Nothing that comes back is executed
Five rules, each with a test:
<title>is somebody else's text and the popover is our document, so it goes in withtextContent. A title of<img src=x onerror=…>draws as those characters, and a test asserts nothing landed onwindowa moment later either.javascript:in a card would run in this origin with this session. An href tampered with in the DOM is still not opened, which is the only reason the second check exists.isImageSource— the same guard a pasted image goes through, and also what stops the browser making any request of its own to the previewed site.eval, nonew Function, no dynamic import. Regex over the head, and the worst case is a panel with no title.rel="noopener noreferrer"on every anchor.The function is not an SSRF hole
A function that fetches a URL from a card makes requests from inside the provider's network on someone else's behalf. "It is only our own users" is not a defence — it is the description of the attack.
guard.jsrefuses:https://docs.example.com@evil.test/reads as the first host and is the second);http://2130706433/to127.0.0.1before the guard ever sees it, which is load-bearing and invisible, so a test pins that too;localhost,.local,.internal,.home.arpa) and any single-label name, since public hostnames have a dot;internal.example.comas an A record for10.0.0.5is the form this actually takes;redirect: 'follow'would let a public URL hop into the private network with nothing looking at it.A refusal answers exactly what a silence answers:
{ ok: false, status: 0 }. Any difference between "not allowed" and "did not reply" makes this a scanner for the network it runs inside. The reason goes to the log and nowhere else.Capped at six seconds, 256KB of page, 120KB of thumbnail, three redirects.
The popover is in the overlay, not in React
Imperatively, with the cursors and the guides. That is the line this app already draws: React renders chrome that shows state — the toolbar, the format bar — and transient pointer-driven chrome lives in the overlay. Being outside React is also what lets the whole thing be driven by a scheduler run by hand and a stubbed fetcher, which is why the states are cheap to test.
Answers are cached per href, so one link asked about twice is one invocation. A failure of ours — function down, network gone — is not cached and says "no preview" rather than "unreachable", because blaming the page for our own outage is a lie the person cannot act on.
Two things the tests found
Both are the "test that cannot fail" shape, and both are worth knowing about this suite:
pointerover. CDP leaves the pointer where the previous test put it, and moving to a position it already occupies dispatches nothing — so two tests were quietly passing against the app's own popover while the instance under test never woke up.hover()now moves off the link first, and this file stands the app's layer down so a gap like that fails loudly.window.openwas stubbed.Also fixed while writing them:
typeof Deno?.resolveDnsstill evaluatesDeno, and an undeclared identifier is a ReferenceError rather than undefined — the resolution guard threw everywhere outside the edge runtime. It would never have fired in production and it lived in a module nothing could import.Tests
878 passing, 96.4% lines. Every new module at 100%:
core/links.js,platform/links.js,platform/link-preview.js,shell/link-preview.js, andcore/bar-position.jsstill.test/node/links.test.js— what counts as a link, where one stops, and what will never be followed.test/node/link-preview.test.js— the boundary where a stranger's page becomes something we draw.test/node/link-preview-function.test.js— the guard and the extraction, including every private range and the parser's IPv4 normalisation.test/node/link-preview-fetch.test.js— the function's fetching half against afetchof our own, including the two SSRF cases the guard alone cannot show.test/browser/links.test.js— the hover, the four states, the cache, and where the panel sits.test/browser/views.test.js,interact.test.js— anchors in every field, hostile text as text, click/drag/shift/editing, and the tampered href.Not in this PR
fetch, which resolves it again. Closing it needs connecting to a checked address while carrying the originalHost, whichfetchcannot express. Also,Deno.resolveDnsis used where it exists; where it does not, the literal and name checks still apply but the resolution check silently does not — I will report which of those is true on the deployed runtime.Deploying
Once this is green I will run
npx supabase functions deploy link-previewagainstvcmmkdrogucgcuatmireand verify from outside that a signed-in call returns a card and a dead URL reports as unreachable. Until then the live site's popover reports previews unavailable, which is the honest state.🤖 Generated with Claude Code
https://claude.ai/code/session_01DGAkrDAKvwwHUFceiFYinQ
Summary by CodeRabbit
wwwlinks in board text.