Weekly review batch: D1 bind-cap crash, missing error toasts, SDK bugs - #57
Weekly review batch: D1 bind-cap crash, missing error toasts, SDK bugs#57DennisAlund wants to merge 6 commits into
Conversation
getStats, getTimeline, getLinkBreakdown, getLinkBreakdownPage, and compareLinkStats each fetched every slug on a link and bound the whole list into a slug IN (?,?,...) clause. D1 caps a prepared statement at 100 bound parameters, so a link with more than ~99 custom slugs (nothing prevents adding that many) failed every one of its analytics queries with SQLITE_ERROR: too many SQL variables. 0.37.1 fixed the identical defect for bundle-scoped queries via bundleSlugScope(), a subquery that resolves membership inside SQLite and costs exactly one bound parameter regardless of member count. This applies the same technique per link (linkSlugScope()) to the five methods above, which had been missed in that pass. Adds a describe block mirroring the existing bundle-cap regression tests: a link with 101 slugs exercises all five methods and asserts correct aggregation instead of a thrown SQLITE_ERROR.
quickShorten, createLink, and createDuplicate parsed the failure body's error field with no .catch() fallback, so a non-JSON error response (an edge-level 502/524 HTML page, a bare 500) rejected the promise unhandled and left the button silent with nothing shown to the user. 0.37.1 added this same fallback to fourteen other handlers but missed these three, the two most common actions in the app (shortening and creating a link). The regression-guard test that is supposed to catch a handler missing this fallback had a blind spot: it matched res.json().then( and toast( on the same source line, but these three handlers spread the call across multiple lines in the project's usual style, so the guard could never see both tokens together. Replaced it with a parser that walks the balanced parentheses of the .then(...) call to find the whole thing regardless of line wrapping, and narrowed it to blocks that read a .error field so it doesn't also flag success-path toasts. Added the three handlers to the existing per-handler empty-body/HTML-body/error-message test matrix.
…oesn't enforce The tool description said "Only the bundle owner can add," but addLinkToBundle() deliberately skips the ownership gate every other bundle-mutation function enforces — adding a link to a bundle is open to any authenticated caller by design (see the comment in bundle-management.ts and the existing 'any authenticated caller can add a link to any bundle' test). An agent trusting the description could believe the call is caller-scoped and add another user's link to a bundle it doesn't own, or reason incorrectly about the security model. Same class of doc/behavior mismatch as the list_bundles fix in 0.37.0. Documentation correction only, no behavior change. Adds a regression test mirroring the existing list_bundles description test.
Both functions built their output on a plain {} target and assigned
converted keys through bracket notation: out[key] = value. When key is
the literal string "__proto__", that assignment doesn't create an own
property on a plain object — it reassigns the object's prototype,
because {} inherits the __proto__ accessor from Object.prototype. A
request body assembled from parsed/untrusted JSON (which, unlike
object-literal syntax, can carry __proto__ as a real own enumerable
property) silently lost that key with no error, e.g.
client.links.update(id, JSON.parse(patch)).
Switched both to Object.create(null) targets, which have no such
accessor to intercept the assignment. Added a focused unit test for
keysToSnake (the exploitable direction: outbound request bodies) and a
client-level integration test showing a __proto__ key now survives
onto the wire. keysToCamel doesn't have a matching direct repro since
its camelCase transform never maps a source key to the literal string
"__proto__", but carries the same fix for symmetry and defense in
depth.
…hing A non-204 2xx response whose body is the literal null (4 bytes) is non-empty and valid JSON, so it passed both existing guards in parse_json_response and returned None. Every single-object resource method's SomeModel.from_dict(...) then crashed on it with a bare AttributeError instead of the documented ShrtnrError — the same failure mode the 1.1.2 empty-body fix exists to prevent, just reached from a non-empty body that happens to parse to None. parse_json_response now raises ShrtnrError when the parsed body is None. Scoped to exactly that value (not every non-dict shape) so list()'s legitimate JSON-array responses are unaffected. Adds a regression test mirroring the existing empty-body test.
BreakdownPage is the return type of both LinksResource.breakdown and BundlesResource.breakdown, shipped in 2.1.0 alongside the other two SDKs, but the Dart README's Key types list never picked it up. TypeScript and Python's READMEs already list BreakdownDimension, BreakdownPage together; this brings Dart's list into parity.
|
Flag: Python SDK
def __init__(
self,
base_url: str,
*,
api_key: str,
...CHANGELOG.md's 1.0.0 entry states: "The positional Not fixing this directly: making Generated by Claude Code |
|
Flag: TypeScript SDK's
Not fixing this directly: if any server-side logic ever keyed off this header (rate-limit exemptions, SDK-vs-browser traffic segmentation), re-adding it is a behavior change that should land with an actual reason, not as an incidental defect-pass fix — and if it was intentionally dropped, the fix is just a changelog correction instead. Needs a call on which it is. Generated by Claude Code |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
shrtnr | 1216f57 | Aug 28 2026, 07:47 PM |
2 participants
Weekly defect-hunting review of the app, its API, and all three SDKs. Six focused passes (API/DB/services, admin UI, MCP/auth, TypeScript SDK, Python SDK, Dart SDK), each finding verified against the actual code and a failing-before/passing-after regression test before being fixed.
Fixes
Per-link analytics crash past D1's 100-bound-parameter cap (
src/db/click-repository.ts)getStats,getTimeline,getLinkBreakdown,getLinkBreakdownPage, andcompareLinkStatseach bound one SQL parameter per slug on a link.SQLITE_ERROR: too many SQL variables. Same class of bug 0.37.1 fixed for bundles viabundleSlugScope(), just missed for the per-link case.linkSlugScope(), the same subquery-based technique, binding one parameter (the link id) regardless of slug count.Missing error toast on the app's two most common actions (
src/client.ts)quickShorten,createLink, andcreateDuplicateparsed the failure body'serrorfield with no.catch()fallback..catch()fallback used everywhere else. Also fixed a blind spot in the regression-guard test itself (it matchedres.json().then(andtoast(on the same source line, which is why these three, spread across multiple lines, slipped through) — replaced it with a paren-balanced scan.MCP
add_link_to_bundledescription claimed an ownership check it doesn't enforce (src/mcp/server.ts)addLinkToBundle()is intentionally open to any authenticated caller (by design, per its own comment and existing test).list_bundlesfix in 0.37.0.list_bundlesdescription regression test.TypeScript SDK:
keysToSnake/keysToCamelsilently dropped a__proto__key (sdk/typescript/src/internal/case.ts){}and assigned viaout[key] = value. Whenkeyis the literal string"__proto__", that reassigns the object's prototype instead of creating an own property.__proto__(e.g.client.links.update(id, JSON.parse(patch))) silently lost that field with no error.Object.create(null)targets.keysToSnake(the exploitable direction) plus a client-level integration test confirming the key survives onto the wire.Python SDK: JSON
null2xx body crashed with a bareAttributeError(sdk/python/src/shrtnr/_base.py)nullis non-empty and valid JSON, so it passed both existing guards inparse_json_responseand returnedNone.SomeModel.from_dict(None)crashed with a bareAttributeErrorinstead of the documentedShrtnrError— the same failure mode the 1.1.2 empty-body fix addressed, just reached from a non-empty body.parse_json_responsenow raisesShrtnrErrorwhen the parsed body isNone, scoped to that value only solist()'s legitimate array responses are unaffected.Doc-only corrections
BreakdownPagefrom its "Key types" list; TypeScript and Python's READMEs already had it. Parity fix.getSparkline's doc comment said 1y buckets are "weekly"; the code (and its own test) have always used monthly, 12-point buckets.Flagged for developer judgment (not fixed, posting as PR comments)
base_urlis documented (CHANGELOG 1.0.0) as keyword-only but is actually positional-or-keyword — fixing would be a public API change.X-Client: sdkheader is sent on every request; it isn't sent anywhere in the current code and was apparently dropped in the 1.0.0 rewrite with no changelog note. Needs a product call on whether to reintroduce it or just correct the historical record.Verification
yarn test: 1299 tests, all green.yarn e2e: confirmed the 15 pre-existing failures in this sandbox reproduce identically on a cleanmaincheckout (proxy-blocked external resources causingERR_CONNECTION_RESET/cert errors) — unrelated to this diff.mypy/ruffclean on changed files.Generated by Claude Code