Code review fixes: ownership gaps, stored XSS, URL normalization, SDK parity

CLAUDE.md

@@ -42,7 +42,7 @@ Each SDK records the SHA-256 of the OpenAPI spec it was last regenerated against
 | Python | `sdk/python/pyproject.toml` | `[tool.shrtnr]` `spec_hash` |
 | Dart | `sdk/dart/pubspec.yaml` | leading comment `# x-spec-hash:` (top-level keys would draw a pana warning) |
 
-Spec changes (edits to `src/api/router.ts`, `src/api/schemas.ts`, or any resource sub-app affecting the generated doc) stale all three hashes. A root `package.json` version bump also drifts the hash because the spec embeds `info.version`. CI enforces via `.github/workflows/sdk-spec-drift.yml`.
+Spec changes (edits to `src/api/router.ts`, `src/api/schemas.ts`, or any resource sub-app affecting the generated doc) stale all three hashes. A root `package.json` version bump also drifts the hash because the spec embeds `info.version`. CI enforces via the `sdk-spec-drift` job in `.github/workflows/ci.yml`.
 
 Workflow on API change:
 

scripts/bump-sdk-version.sh

@@ -170,8 +170,9 @@ echo "  1. edit $CHANGELOG and replace the TODO placeholder"
 echo "  2. git add $MANIFEST $CHANGELOG"
 echo "  3. git commit -m \"Release $SDK $NEW_VERSION: <short summary>\""
 if [ "$SDK" = "pub" ]; then
-  echo "  4. git tag ${TAG_PREFIX}${NEW_VERSION}"
-  echo "  5. git push origin main ${TAG_PREFIX}${NEW_VERSION}"
+  echo "  4. git tag ${TAG_PREFIX}${NEW_VERSION}   (create locally, do not push yet)"
+  echo "     pub.dev publishes on tag push, so push the tag only when you are"
+  echo "     ready to release: git push origin ${TAG_PREFIX}${NEW_VERSION}"
 else
   echo "  4. gh pr create  (or push to main — CI tags after publish)"
 fi

sdk/dart/README.md

@@ -50,7 +50,7 @@ closed by `client.close()`.
 | `get(id, {range?})` | Get a link with click count |
 | `list({owner?, range?})` | List all links |
 | `create({url, label?, slugLength?, expiresAt?, allowDuplicate?})` | Create a short link |
-| `update(id, {url?, label?, expiresAt?})` | Update URL, label, or expiry |
+| `update(link)` | Update URL, label, or expiry (pass a `Link` from `copyWith`) |
 | `disable(id)` | Stop redirecting |
 | `enable(id)` | Resume redirecting |
 | `delete(id)` | Permanently delete |
@@ -65,10 +65,10 @@ closed by `client.close()`.
 final link = await client.links.create(url: 'https://example.com', label: 'Landing page');
 
 // Get a 7-day click count
-final fresh = await client.links.get(link.id, range: '7d');
+final fresh = await client.links.get(link.id, range: TimelineRange.last7d);
 
 // Full analytics for the last 30 days
-final stats = await client.links.analytics(link.id, range: '30d');
+final stats = await client.links.analytics(link.id, range: TimelineRange.last30d);
 print('${stats.totalClicks} clicks, ${stats.numCountries} countries');
 ```
 
@@ -100,7 +100,7 @@ Groups of related links with combined analytics.
 | `get(id, {range?})` | Get a bundle with click summary |
 | `list({archived?, range?})` | List bundles |
 | `create({name, description?, icon?, accent?})` | Create a bundle |
-| `update(id, {name?, description?, icon?, accent?})` | Update metadata |
+| `update(bundle)` | Update metadata (pass a `Bundle` from `copyWith`) |
 | `delete(id)` | Permanently delete |
 | `archive(id)` | Hide from default listing |
 | `unarchive(id)` | Restore an archived bundle |
@@ -112,12 +112,12 @@ Groups of related links with combined analytics.
 
 ```dart
 // Create a bundle and add links to it
-final bundle = await client.bundles.create(name: 'Spring 2026', accent: 'green');
+final bundle = await client.bundles.create(name: 'Spring 2026', accent: BundleAccent.green);
 await client.bundles.addLink(bundle.id, linkA.id);
 await client.bundles.addLink(bundle.id, linkB.id);
 
 // Combined analytics for the last 7 days
-final stats = await client.bundles.analytics(bundle.id, range: '7d');
+final stats = await client.bundles.analytics(bundle.id, range: TimelineRange.last7d);
 print(stats.totalClicks);
 ```
 
@@ -130,8 +130,9 @@ Key types exported from `package:shrtnr/shrtnr.dart`:
 
 - `Link`, `Slug`, `Bundle`, `BundleWithSummary`, `BundleTopLink`
 - `ClickStats`, `TimelineData`, `TimelineBucket`, `TimelineSummary`, `NameCount`
-- `DateClickCount`, `SlugClickCount`
+- `DateCount`, `SlugCount`
 - `DeletedResult`, `AddedResult`, `RemovedResult`
+- Enums: `TimelineRange`, `BundleAccent`, `BreakdownDimension`, `BundleArchivedFilter`
 
 Timestamp fields (`createdAt`, `expiresAt`, `disabledAt`, `archivedAt`, `updatedAt`) are
 plain `int` Unix seconds, matching the wire format exactly.

sdk/dart/lib/src/errors.dart

@@ -19,5 +19,5 @@ class ShrtnrError implements Exception {
   final String serverMessage;
 
   @override
-  String toString() => 'ShrtnrError(HTTP $status): $serverMessage';
+  String toString() => 'shrtnr API error (HTTP $status): $serverMessage';
 }

sdk/dart/lib/src/models.dart

@@ -102,14 +102,17 @@ enum TimelineRange {
 /// uses [trueValue] and [activeOnly]. The wire value `"1"` is omitted because
 /// it is a semantic alias for `"true"`; use [trueValue] for both.
 enum BundleArchivedFilter {
-  /// Include archived bundles alongside active ones (wire: `"true"`). Also
-  /// covers the `"1"` alias from the spec; prefer this member for both.
+  /// Return only archived bundles (wire: `"true"`). Also covers the `"1"`
+  /// alias from the spec; prefer this member for both.
   trueValue,
 
-  /// Return only archived bundles (wire: `"only"`).
+  /// Return only archived bundles (wire: `"only"`). Despite the name, the
+  /// server's `"only"` value returns archived bundles only, the same result
+  /// as [trueValue]. The name is misleading and is kept for wire
+  /// compatibility; a rename awaits the next major version.
   activeOnly,
 
-  /// Return all bundles regardless of archived status (wire: `"all"`).
+  /// Include archived bundles alongside active ones (wire: `"all"`).
   all;
 
   static const _wireValues = {

sdk/python/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 All notable changes to the SDK are documented in this file.
 
+## 1.1.1
+
+- `links.qr()` now takes `size` as an `int` instead of a `str`, matching the API schema (which validates an integer) and the TypeScript and Dart SDKs. Callers who passed an int already got the correct behavior; the annotation was the only thing out of step.
+
 ## 1.1.0 (2026-06-19)
 
 - Add `links.breakdown` and `bundles.breakdown` (sync and async) for paging through the countries, sources and domains analytics panels (offset/limit, returns items + total).

sdk/python/README.md

@@ -65,7 +65,7 @@ with Shrtnr(base_url="...", api_key="sk_...") as client:
 | `get(id, *, range=None)` | Get a link with click count |
 | `list(*, owner=None, range=None)` | List all links |
 | `create(*, url, label=None, slug_length=None, expires_at=None, allow_duplicate=None)` | Create a short link |
-| `update(id, *, url=None, label=None, expires_at=None)` | Update URL, label, or expiry |
+| `update(id, *, url=None, label=UNSET, expires_at=UNSET)` | Update URL, label, or expiry. Omit a field to leave it unchanged; pass `None` to clear it |
 | `disable(id)` | Stop redirecting |
 | `enable(id)` | Resume redirecting |
 | `delete(id)` | Permanently delete |
@@ -115,7 +115,7 @@ Groups of related links with combined analytics.
 | `get(id, *, range=None)` | Get a bundle with click summary |
 | `list(*, archived=None, range=None)` | List bundles |
 | `create(*, name, description=None, icon=None, accent=None)` | Create a bundle |
-| `update(id, *, name=None, description=None, icon=None, accent=None)` | Update metadata |
+| `update(id, *, name=None, description=UNSET, icon=UNSET, accent=None)` | Update metadata. Omit a field to leave it unchanged; pass `None` to clear `description` or `icon` |
 | `delete(id)` | Permanently delete |
 | `archive(id)` | Hide from default listing |
 | `unarchive(id)` | Restore an archived bundle |

sdk/python/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
 
 [project]
 name = "shrtnr"
-version = "1.1.0"
+version = "1.1.1"
 description = "SDK for the shrtnr URL shortener API"
 readme = "README.md"
 license = "Apache-2.0"

sdk/python/src/shrtnr/resources/links.py

@@ -171,13 +171,13 @@ def timeline(self, id: int, *, range: TimelineRange | None = None) -> TimelineDa
         url = self._url(f"/_/api/links/{id}/timeline", {"range": range})
         return TimelineData.from_dict(self._request("GET", url, headers=self._headers()))
 
-    def qr(self, id: int, *, slug: str | None = None, size: str | None = None) -> str:
+    def qr(self, id: int, *, slug: str | None = None, size: int | None = None) -> str:
         """Get the QR code SVG for a link. Returns the SVG string."""
         query: dict[str, str | None] = {}
         if slug is not None:
             query["slug"] = slug
         if size is not None:
-            query["size"] = size
+            query["size"] = str(size)
         url = self._url(f"/_/api/links/{id}/qr", query or None)
         return self._request_text("GET", url, headers=self._headers())
 
@@ -327,13 +327,13 @@ async def timeline(self, id: int, *, range: TimelineRange | None = None) -> Time
         url = self._url(f"/_/api/links/{id}/timeline", {"range": range})
         return TimelineData.from_dict(await self._request("GET", url, headers=self._headers()))
 
-    async def qr(self, id: int, *, slug: str | None = None, size: str | None = None) -> str:
+    async def qr(self, id: int, *, slug: str | None = None, size: int | None = None) -> str:
         """Get the QR code SVG for a link. Returns the SVG string."""
         query: dict[str, str | None] = {}
         if slug is not None:
             query["slug"] = slug
         if size is not None:
-            query["size"] = size
+            query["size"] = str(size)
         url = self._url(f"/_/api/links/{id}/qr", query or None)
         return await self._request_text("GET", url, headers=self._headers())
 

sdk/python/tests/test_client.py

@@ -341,7 +341,7 @@ def test_links_qr_with_slug_and_size(client: Shrtnr) -> None:
     route = respx.get(url__regex=rf"^{BASE_URL}/_/api/links/5/qr\?").mock(
         return_value=httpx.Response(200, text="<svg/>"),
     )
-    client.links.qr(5, slug="promo", size="200")
+    client.links.qr(5, slug="promo", size=200)
     url = str(route.calls[0].request.url)
     assert "slug=promo" in url
     assert "size=200" in url

src/__tests__/handler/mcp.test.ts

@@ -235,7 +235,7 @@ describe("MCP tool behavior (service layer)", () => {
 
     const result = await updateLink(env as never, created.data.id, {
       url: "https://example.com/updated",
-    });
+    }, "anonymous");
     expect(result.ok).toBe(true);
     if (result.ok) {
       expect(result.data.url).toBe("https://example.com/updated");

src/__tests__/page/keys-page.test.ts

@@ -113,6 +113,13 @@ describe("Keys page table", () => {
   it("keeps the delete action for each key", async () => {
     await seedApiKey();
     const html = await fetchKeysHtml();
-    expect(html).toContain("deleteKey(");
+    // The delete action rides on data-* attributes read by a delegated handler
+    // rather than an inline onclick, so a key title cannot break out into
+    // executable script. The title is carried verbatim on data-delete-key-title.
+    expect(html).toMatch(/data-delete-key="\d+"/);
+    expect(html).toContain('data-delete-key-title=');
+    // The vulnerable inline onclick handler must be gone from the markup. The
+    // client script still defines deleteKey(), so target the attribute form.
+    expect(html).not.toContain('onclick="deleteKey(');
   });
 });

src/__tests__/page/link-detail-page.test.ts

@@ -44,6 +44,27 @@ describe("Link detail page server render", () => {
     expect(slugCount![1].trim()).toBe("2");
   });
 
+  it("renders the duplicate action without interpolating the URL into an inline handler", async () => {
+    // A URL with a literal single quote must not break out of the duplicate
+    // button. The URL rides on a data-* attribute (HTML-escaped by JSX) and a
+    // delegated handler reads it, so no inline onclick carries the raw URL.
+    const link = await LinkRepository.create(env.DB, {
+      url: "https://example.com/'-alert(document.cookie)-'",
+      slug: "abc",
+    });
+
+    const res = await SELF.fetch(req(`/_/admin/links/${link.id}`));
+    const html = await res.text();
+
+    expect(html).toContain("data-duplicate-url=");
+    // The client script still defines showDuplicateModal(); the fix is that no
+    // inline onclick attribute carries the raw URL.
+    expect(html).not.toContain('onclick="showDuplicateModal(');
+    // The quote survives only in escaped form, never as a raw breakout.
+    expect(html).not.toContain("'-alert(document.cookie)-'");
+    expect(html).toContain("'-alert(document.cookie)-'");
+  });
+
   it("hero total_clicks honors a user's default_range setting", async () => {
     const link = await LinkRepository.create(env.DB, { url: "https://example.com", slug: "abc" });
     const slug = link.slugs[0].slug;

src/__tests__/service/link-service.test.ts

@@ -120,7 +120,7 @@ describe("link-management service", () => {
     expect(created.ok).toBe(true);
     if (!created.ok) return;
 
-    const result = await updateLink(env as any, created.data.id, { url: "javascript:alert(1)" });
+    const result = await updateLink(env as any, created.data.id, { url: "javascript:alert(1)" }, "anonymous");
     expect(result.ok).toBe(false);
     if (!result.ok) {
       expect(result.status).toBe(400);
@@ -135,7 +135,7 @@ describe("link-management service", () => {
     const fetched = await getLink(env as any, created.data.id);
     expect(fetched.ok).toBe(true);
 
-    const updated = await updateLink(env as any, created.data.id, { label: "Updated" });
+    const updated = await updateLink(env as any, created.data.id, { label: "Updated" }, "anonymous");
     expect(updated.ok).toBe(true);
     if (updated.ok) {
       expect(updated.data.label).toBe("Updated");
@@ -307,7 +307,7 @@ describe("URL normalization in updateLink", () => {
     expect(created.ok).toBe(true);
     if (!created.ok) return;
 
-    const updated = await updateLink(env as any, created.data.id, { url: "https://example.com/new-path/" });
+    const updated = await updateLink(env as any, created.data.id, { url: "https://example.com/new-path/" }, "anonymous");
     expect(updated.ok).toBe(true);
     if (updated.ok) {
       expect(updated.data.url).toBe("https://example.com/new-path");
@@ -319,7 +319,7 @@ describe("URL normalization in updateLink", () => {
     expect(created.ok).toBe(true);
     if (!created.ok) return;
 
-    const updated = await updateLink(env as any, created.data.id, { url: "https://example.com/page?" });
+    const updated = await updateLink(env as any, created.data.id, { url: "https://example.com/page?" }, "anonymous");
     expect(updated.ok).toBe(true);
     if (updated.ok) {
       expect(updated.data.url).toBe("https://example.com/page");
@@ -331,7 +331,7 @@ describe("URL normalization in updateLink", () => {
     expect(created.ok).toBe(true);
     if (!created.ok) return;
 
-    const updated = await updateLink(env as any, created.data.id, { label: null });
+    const updated = await updateLink(env as any, created.data.id, { label: null }, "anonymous");
     expect(updated.ok).toBe(true);
     if (updated.ok) {
       expect(updated.data.label).toBeNull();
@@ -389,7 +389,7 @@ describe("field type validation in updateLink", () => {
     expect(created.ok).toBe(true);
     if (!created.ok) return;
 
-    const updated = await updateLink(env as any, created.data.id, { expires_at: "never" as any });
+    const updated = await updateLink(env as any, created.data.id, { expires_at: "never" as any }, "anonymous");
     expect(updated.ok).toBe(false);
     if (!updated.ok) expect(updated.status).toBe(400);
   });
@@ -399,7 +399,7 @@ describe("field type validation in updateLink", () => {
     expect(created.ok).toBe(true);
     if (!created.ok) return;
 
-    const updated = await updateLink(env as any, created.data.id, { label: 42 as any });
+    const updated = await updateLink(env as any, created.data.id, { label: 42 as any }, "anonymous");
     expect(updated.ok).toBe(false);
     if (!updated.ok) expect(updated.status).toBe(400);
   });
@@ -409,7 +409,7 @@ describe("field type validation in updateLink", () => {
     expect(created.ok).toBe(true);
     if (!created.ok) return;
 
-    const updated = await updateLink(env as any, created.data.id, { expires_at: null });
+    const updated = await updateLink(env as any, created.data.id, { expires_at: null }, "anonymous");
     expect(updated.ok).toBe(true);
     if (updated.ok) expect(updated.data.expires_at).toBeNull();
   });

src/__tests__/service/ownership.test.ts

@@ -12,6 +12,8 @@ import {
   enableSlug,
   removeSlug,
   addCustomSlugToLink,
+  updateLink,
+  setSlugPrimary,
   searchLinks,
   listLinksByOwner,
 } from "../../services/link-management";
@@ -91,6 +93,48 @@ describe("Link ownership: delete", () => {
   });
 });
 
+describe("Link ownership: update", () => {
+  it("owner can update their link", async () => {
+    const link = await createOwnedLink();
+    const result = await updateLink(env as any, link.id, { url: "https://example.com/moved" }, OWNER);
+    expect(result.ok).toBe(true);
+    if (result.ok) {
+      expect(result.data.url).toBe("https://example.com/moved");
+    }
+  });
+
+  it("non-owner cannot update another user's link", async () => {
+    const link = await createOwnedLink();
+    const result = await updateLink(env as any, link.id, { url: "https://evil.example" }, OTHER);
+    expect(result.ok).toBe(false);
+    if (!result.ok) {
+      expect(result.status).toBe(403);
+    }
+    // The destination must be untouched after a rejected update.
+    const after = await LinkRepository.getById(env.DB, link.id);
+    expect(after!.url).toBe("https://example.com");
+  });
+});
+
+describe("Slug ownership: set primary", () => {
+  it("link owner can change the primary slug", async () => {
+    const link = await createOwnedLink();
+    await addCustomSlugToLink(env as any, link.id, { slug: "primary-pick" });
+    const result = await setSlugPrimary(env as any, link.id, "primary-pick", OWNER);
+    expect(result.ok).toBe(true);
+  });
+
+  it("non-owner cannot change the primary slug on another user's link", async () => {
+    const link = await createOwnedLink();
+    await addCustomSlugToLink(env as any, link.id, { slug: "primary-pick" });
+    const result = await setSlugPrimary(env as any, link.id, "primary-pick", OTHER);
+    expect(result.ok).toBe(false);
+    if (!result.ok) {
+      expect(result.status).toBe(403);
+    }
+  });
+});
+
 describe("Slug ownership: disable", () => {
   it("link owner can disable a custom slug", async () => {
     const link = await createOwnedLink();

src/__tests__/unit/normalize-url.test.ts

@@ -63,4 +63,22 @@ describe("normalizeUrl", () => {
       "https://example.com#section",
     );
   });
+
+  it("preserves a trailing slash inside a query value", () => {
+    expect(normalizeUrl("https://example.com/search?q=cats/")).toBe(
+      "https://example.com/search?q=cats/",
+    );
+  });
+
+  it("preserves a trailing slash inside an embedded URL parameter", () => {
+    expect(normalizeUrl("https://example.com/a?next=https://b.com/")).toBe(
+      "https://example.com/a?next=https://b.com/",
+    );
+  });
+
+  it("preserves a hash-router route that ends in a slash", () => {
+    expect(normalizeUrl("https://example.com/#/spa/route/")).toBe(
+      "https://example.com/#/spa/route/",
+    );
+  });
 });