Resolve known link-shortener URLs (search.app / share.google) to their destination

[bcamarneiro]

Description

When a bookmark is saved as a Google short/share link (https://search.app/... or https://share.google/...), the crawler already follows the redirect to fetch and archive the real page — but the stored bookmark URL keeps the opaque short link. This means the entry never reflects the actual destination, and if the shortener ever stops resolving, the real URL is lost.

This PR overwrites the stored URL with the URL the crawler actually resolved to, but only when:

• the original host is a known link-shortener (search.app, share.google), and
• the crawler ended up at a different URL, and
• that destination is a safe http(s) URL.

Scoping it to a small allowlist (rather than following every redirect) means ordinary redirects — http→https, tracking-param strips, trailing-slash canonicalization — never silently rewrite the user's URL. The safe-scheme guard (reusing the existing isAllowedBookmarkUrl) ensures a shortener can never rewrite a bookmark to a javascript:/data:/file: URL.

The decision logic is extracted into a pure, unit-tested helper resolveShortenedBookmarkUrl(originalUrl, crawledUrl) in packages/shared/utils/url.ts. New shorteners can be added by extending the KNOWN_LINK_SHORTENERS list.

Resolution is applied consistently across all three outcomes of a crawl:

• HTML pages — folded into the existing bookmarkLinks update (no new query, no DB migration), using the browser's final URL (robust to client-side redirects).
• Direct PDF/image links — the content-type probe now returns the redirect's final URL, so the asset-bookmark path stores the resolved destination as sourceUrl instead of the short link.
• Downstream jobs — crawlAndParseUrl returns the effective (post-resolution) URL so the video-download queue receives the real destination, not the opaque short link.

Fixes #2235

How Has This Been Tested?

• Unit tests for isKnownLinkShortener and resolveShortenedBookmarkUrl in packages/shared/utils/url.test.ts, covering: known-shortener → resolved URL returned; non-shortener → unchanged; no-op redirect → unchanged; dangerous-scheme destination rejected; lookalike hosts (search.app.evil.com, notsearch.app, www.search.app) rejected. (18/18 passing.)
• e2e tests in packages/e2e_tests — the nginx fixture container is aliased as search.app (allow-listed as a known shortener) with /shortlink and /shortlink-image redirects, asserting the stored url (HTML path) and sourceUrl (asset path) are the resolved destinations, not the short link. Run locally against the full container stack: 7/7 crawler tests passing (http://search.app/shortlink → stored .../hello.html; http://search.app/shortlink-image → sourceUrl .../image.png).
• pnpm typecheck, pnpm lint, and pnpm format pass across the monorepo.

Checklist:

• I have carefully read CONTRIBUTING.md
• I have performed a self-review of my own code
• I have made corresponding changes to the documentation if applicable
• I have no unrelated changes in the PR.
• I have confirmed that any new dependencies are strictly necessary. (No new dependencies.)
• I have written tests for new code (if applicable)

Please describe to which degree, if any, an LLM was used in creating this pull request.

This PR was developed with the help of an AI coding assistant, following a test-driven approach (tests written and watched to fail before the implementation). All code was reviewed, run, and verified by me before submission.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[greptile-apps]

Greptile Summary

This PR resolves known link-shortener URLs (search.app, share.google) to their crawled destinations by overwriting bookmarkLinks.url during Phase 1 of the crawl, guarded by an exact-host allowlist and a safe-scheme check via the existing isAllowedBookmarkUrl. The helper logic is extracted into a pure, well-tested function in packages/shared/utils/url.ts.

• resolveShortenedBookmarkUrl returns the resolved URL only when the original host is on the KNOWN_LINK_SHORTENERS allowlist, the crawler landed somewhere different, and the destination passes the http/https scheme guard — keeping benign redirects from silently rewriting user URLs.
• 18 unit tests cover exact-host matching, lookalike rejection, dangerous-scheme blocking, and no-op redirects.
• One gap: the outer runCrawler url variable is not updated after crawlAndParseUrl resolves the shortener, so VideoWorkerQueue.enqueue still receives the original opaque shortener URL instead of the resolved destination.

Confidence Score: 3/5

Safe to merge for the core use-case, but video downloads will silently use the wrong URL for shortener-originated bookmarks.

The URL helper and its tests are correct and conservative. The crawler integration also works correctly for the main bookmark-URL field. However, after crawlAndParseUrl writes the resolved URL to the DB, the url local variable in runCrawler still holds the original shortener URL. VideoWorkerQueue.enqueue at line 2447 receives that stale shortener URL — not the resolved destination — so any video linked through a shortener would fail to download or be silently skipped by the video worker.

apps/workers/workers/crawlerWorker.ts — specifically the VideoWorkerQueue.enqueue call and the url variable lifetime in runCrawler after crawlAndParseUrl returns.

Important Files Changed

Filename	Overview
apps/workers/workers/crawlerWorker.ts	Adds resolved-URL overwrite for known shorteners in Phase 1 DB write, but the outer runCrawler url variable is never refreshed, causing the video-download queue to receive the original shortener URL instead of the resolved destination.
packages/shared/utils/url.ts	Adds KNOWN_LINK_SHORTENERS, isKnownLinkShortener, and resolveShortenedBookmarkUrl with exact-host matching and safe-scheme guard; logic is correct and minimal.
packages/shared/utils/url.test.ts	18 well-targeted unit tests covering known shorteners, lookalike hosts, no-op redirects, and dangerous-scheme destinations; all relevant edge cases are represented.

Comments Outside Diff (1)

1. apps/workers/workers/crawlerWorker.ts, line 2444-2450 (link)

   Stale shortener URL passed to video worker

   crawlAndParseUrl writes the resolved destination URL to bookmarkLinks.url in the DB, but the url local variable in runCrawler is never updated. When a known shortener (e.g. search.app) resolves to a video page, VideoWorkerQueue.enqueue receives the original opaque shortener URL — not the resolved destination — so the video download worker will try to process the shortener URL and likely fail or silently skip the video entirely.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/workers/workers/crawlerWorker.ts
   Line: 2444-2450
   
   Comment:
   **Stale shortener URL passed to video worker**
   
   `crawlAndParseUrl` writes the resolved destination URL to `bookmarkLinks.url` in the DB, but the `url` local variable in `runCrawler` is never updated. When a known shortener (e.g. `search.app`) resolves to a video page, `VideoWorkerQueue.enqueue` receives the original opaque shortener URL — not the resolved destination — so the video download worker will try to process the shortener URL and likely fail or silently skip the video entirely.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/workers/workers/crawlerWorker.ts:2444-2450
**Stale shortener URL passed to video worker**

`crawlAndParseUrl` writes the resolved destination URL to `bookmarkLinks.url` in the DB, but the `url` local variable in `runCrawler` is never updated. When a known shortener (e.g. `search.app`) resolves to a video page, `VideoWorkerQueue.enqueue` receives the original opaque shortener URL — not the resolved destination — so the video download worker will try to process the shortener URL and likely fail or silently skip the video entirely.

_{Reviews (1): Last reviewed commit: "Resolve known link-shortener URLs to the..." | Re-trigger Greptile}

[bcamarneiro]

Good catch on the stale local url in runCrawler — I've addressed it in b6d08fa.

One clarification on the stated impact, though: the runtime effect on video downloads is nil today. videoWorker doesn't read url from the queue payload — it re-fetches it from the DB via getBookmarkDetails(bookmarkId) (videoWorker.ts:108-112), and by the time the job is enqueued the resolved URL has already been written to bookmarkLinks.url (the Phase-1 await db.update(...) completes before crawlAndParseUrl returns). It then resolves redirects itself via resolveValidatedRedirectUrl. So the payload url was effectively dead data.

That said, passing a stale value is a smell worth removing, so crawlAndParseUrl now returns the effective (post-resolution) URL and the video enqueue uses it — the payload is accurate and won't mislead if something starts trusting it later.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[pambroziak-rtbh]

@MohamedBassem any chance to get this merged? This feature is a game changer for using Karakeep with the Google Discovery feed

[bcamarneiro]

Rebased onto current main — the branch is conflict-free again. Conflicts were mechanical: an import in crawlAndParse.ts and the CRAWLER_ALLOWED_INTERNAL_HOSTNAMES line in the e2e docker-compose.yml, which now carries both search.app and the upstream CRAWLER_STORE_PDF addition. No behaviour change from the previous review round.

@MohamedBassem this is ready whenever you have a moment.