Developer platform, SDK packages, and docs overhaul#1639
Developer platform, SDK packages, and docs overhaul#1639richiemcilroy merged 82 commits intomainfrom
Conversation
richiemcilroy
commented
Mar 2, 2026
richiemcilroy
commented
- Developer platform with app management, API keys, credits, and usage billing
- SDK packages (@cap/sdk-embed, @cap/sdk-recorder) for third-party integrations
- New docs site with sidebar navigation, search, and syntax highlighting
- Developer dashboard section with apps, credits, and usage pages
|
Too many files changed for review. ( |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const plainContent = doc.content | ||
| .replace(/---[\s\S]*?---/, "") | ||
| .replace(/```[\s\S]*?```/g, "") | ||
| .replace(/<[^>]+>/g, "") |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
This autofix suggestion was applied.
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, to fix incomplete multi‑character sanitization you either (1) switch to a robust sanitization/escaping library, or (2) ensure that all potentially dangerous characters or patterns are removed/escaped in a way that cannot be undone by previous multi‑character replacements, for example by matching single characters (</>) or by escaping them so they are no longer syntactically meaningful in HTML.
For this specific code, the simplest and safest adjustment—without changing functional intent—is to make plainContent truly “plain text” by stripping all < and > characters after removing Markdown and tags. This way, even if /<[^>]+>/g misses something and leaves a fragment like <script or </, the final .replace(/[<>]/g, "") (or similar) ensures no residual < or > remain, eliminating HTML tag injection via this field. This also directly addresses the CodeQL concern about <script persisting and avoids multi‑character sanitization pitfalls by operating on single characters.
Concretely, in apps/web/utils/docs.ts, inside getDocSearchIndex, update the plainContent construction to add an extra .replace(/[<>]/g, "") (or merge this into the existing Markdown‑character removal) after the HTML tag stripping and Markdown symbol removal. No extra imports are needed. All other logic (front‑matter removal, code block removal, whitespace normalization, trimming, and slicing to 500 chars) stays unchanged.
| @@ -130,6 +130,7 @@ | ||
| .replace(/```[\s\S]*?```/g, "") | ||
| .replace(/<[^>]+>/g, "") | ||
| .replace(/[#*`[\]()]/g, "") | ||
| .replace(/[<>]/g, "") | ||
| .replace(/\n+/g, " ") | ||
| .trim() | ||
| .slice(0, 500); |
richiemcilroy
committed this autofix suggestion 1 day ago.
…cter sanitization Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Paragon SummaryThis pull request review analyzed 100 files and found no issues. The review examined code changes, potential bugs, security vulnerabilities, performance issues, and code quality concerns using automated analysis tools. Paragon did not detect any problems in the current diff. Proceed with merge after your normal checks. This PR introduces a developer platform with app management, API keys, credits, and usage billing, along with SDK packages for third-party integrations and a redesigned documentation site with improved navigation and search. Key changes:
Confidence score: 5/5
100 files reviewed, 0 comments |
![paragon-review[bot]](https://avatars.githubusercontent.com/in/1544258?s=60&v=4){kind=small}
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| const [result] = await db() | ||
| .update(developerCreditAccounts) | ||
| .set({ | ||
| balanceMicroCredits: sql`${developerCreditAccounts.balanceMicroCredits} - ${MIN_BALANCE_MICRO_CREDITS}`, |
There was a problem hiding this comment.
This now debits MIN_BALANCE_MICRO_CREDITS inside /videos/create.
If /upload/multipart/complete is also debiting based on durationInSecs, this will double-charge (and it also charges even if the upload never completes / gets aborted). If the intent was just an atomic “has at least X credits” check, I’d switch this back to a read/check (or implement an explicit reservation + reconciliation/refund flow).
| return c.json({ error: "Credit account not found" }, 402); | ||
| } | ||
|
|
||
| const debited = await db().transaction(async (tx) => { |
There was a problem hiding this comment.
Right now credits are debited (and a transaction is inserted) before bucket.multipart.complete(...).
That means transient S3/MinIO failures can charge users for uploads that never successfully finalize. Safer pattern is to complete the multipart first and only then debit, or add a compensating refund/rollback path if completion fails after debiting.
|
|
||
| const rateLimiter = createMiddleware(async (c, next) => { | ||
| const key = | ||
| c.req.header("authorization")?.slice(0, 20) ?? |
There was a problem hiding this comment.
The in-memory rate limit is a good start, but a couple sharp edges:
requestCountscan grow unbounded with many unique keys (no TTL cleanup).x-forwarded-foris often a comma-separated list; using it verbatim (or as a fallback for unauth’d requests) can produce huge/unique keys.authorization?.slice(0, 20)risks collisions across API keys.
Might be worth parsing the first IP, and adding some pruning/max-size behavior so long-lived instances don’t accumulate keys indefinitely.
| videoTrack.onended = () => { | ||
| this.stop(); | ||
| }; |
There was a problem hiding this comment.
These callbacks call an async stop() but don’t handle the returned promise. That can turn into unhandled rejections (and makes failures hard to debug).
| videoTrack.onended = () => { | |
| this.stop(); | |
| }; | |
| videoTrack.onended = () => { | |
| void this.stop().catch((err) => { | |
| console.error("CapRecorder: stop failed:", err); | |
| }); | |
| }; |
| this.maxDurationTimeout = setTimeout(() => { | ||
| this.stop(); | ||
| }, this.maxDurationMs); |
There was a problem hiding this comment.
Same async stop() issue here.
| this.maxDurationTimeout = setTimeout(() => { | |
| this.stop(); | |
| }, this.maxDurationMs); | |
| this.maxDurationTimeout = setTimeout(() => { | |
| void this.stop().catch((err) => { | |
| console.error("CapRecorder: stop failed:", err); | |
| }); | |
| }, this.maxDurationMs); |
| this._videoId = videoId; | ||
|
|
||
| this._durationMs = | ||
| this.accumulatedMs + (Date.now() - this.lastResumeTime); |
There was a problem hiding this comment.
Since durationMs now feeds into billing (durationInSecs in the multipart complete call), be careful with paused recordings: when you stop from the paused phase, Date.now() - this.lastResumeTime will include the paused time.
It might be worth computing the final duration in stop() (before changing phases) and reusing that value during upload, plus optionally pausing/restarting maxDurationTimeout so maxDurationMs is based on recording time rather than wall time.
| export default async function DevVideoPage({ | ||
| params, | ||
| }: { | ||
| params: Promise<{ videoId: string }>; |
There was a problem hiding this comment.
params isn’t a Promise in Next.js app routes (and awaiting a plain object is a bit odd). This can be simplified.
| params: Promise<{ videoId: string }>; | |
| export default function DevVideoPage({ | |
| params, | |
| }: { | |
| params: { videoId: string }; | |
| }) { | |
| redirect(`/embed/${params.videoId}?sdk=1`); | |
| } |
| let rateLimitRequestCounter = 0; | ||
|
|
||
| export const developerRateLimiter = createMiddleware(async (c, next) => { | ||
| const key = |
There was a problem hiding this comment.
For the rate limit key, x-forwarded-for is often a comma-separated list; using it verbatim can create lots of unique keys.
| const key = | |
| const authorization = c.req.header("authorization")?.split(" ")[1]; | |
| const forwardedFor = c.req.header("x-forwarded-for")?.split(",")[0]?.trim(); | |
| const key = authorization ?? forwardedFor ?? "unknown"; |
| (_, j) => i + j, | ||
| ); | ||
|
|
||
| const results = await Promise.all( |
There was a problem hiding this comment.
With concurrency, it’s easy to end up with partial success + an early throw (presign/fetch), and aborting from within each task can race. A simple pattern is: try the whole batch, abort once on any failure.
| const results = await Promise.all( | |
| let results: typeof completedParts; | |
| try { | |
| results = await Promise.all( | |
| batch.map(async (idx) => { | |
| const start = idx * CHUNK_SIZE; | |
| const end = Math.min(start + CHUNK_SIZE, blob.size); | |
| const chunk = blob.slice(start, end); | |
| const partNumber = idx + 1; | |
| const { presignedUrl } = await this.request( | |
| "/upload/multipart/presign-part", | |
| { | |
| videoId, | |
| uploadId, | |
| partNumber, | |
| }, | |
| ); | |
| const uploadResponse = await fetch(presignedUrl, { | |
| method: "PUT", | |
| body: chunk, | |
| }); | |
| if (!uploadResponse.ok) { | |
| throw new Error(`Failed to upload part ${partNumber}`); | |
| } | |
| const etag = uploadResponse.headers.get("ETag") ?? ""; | |
| return { | |
| partNumber, | |
| etag, | |
| size: end - start, | |
| }; | |
| }), | |
| ); | |
| } catch (err) { | |
| await this.request("/upload/multipart/abort", { videoId, uploadId }); | |
| throw err; | |
| } |
| const s3Key = video.s3Key; | ||
|
|
||
| try { | ||
| const totalBytes = parts.reduce((sum, p) => sum + p.size, 0); |
There was a problem hiding this comment.
totalBytes / parts[].size comes from the client, so it’s easy to under-report and bypass the size-based minimum. If this is for abuse/billing, it’s safer to derive size from S3 (e.g. HEAD the object after multipart.complete, or query multipart part sizes server-side). Also note you debit credits before multipart.complete, so failures here will still charge users unless you add a compensating refund path.
|
|
||
| export const STRIPE_DEVELOPER_CREDITS_PRODUCT_ID: Record<string, string> = { | ||
| development: "prod_U4mswfBp0bFc39", | ||
| production: "prod_REPLACE_BEFORE_PRODUCTION", |
There was a problem hiding this comment.
prod_REPLACE_BEFORE_PRODUCTION looks like an easy footgun to ship. Might be worth failing fast in production if this placeholder is still present (so it doesn’t turn into a confusing Stripe error at runtime).
| const DEVELOPER_DASHBOARD_ALLOWED_EMAILS = ["richie@cap.so"]; | ||
|
|
||
| const showDeveloperDashboard = | ||
| buildEnv.NEXT_PUBLIC_IS_CAP && | ||
| DEVELOPER_DASHBOARD_ALLOWED_EMAILS.includes(user.email); |
There was a problem hiding this comment.
Might be worth normalizing user.email (and guarding for null/undefined) to avoid an unexpected crash / mismatch.
| const DEVELOPER_DASHBOARD_ALLOWED_EMAILS = ["richie@cap.so"]; | |
| const showDeveloperDashboard = | |
| buildEnv.NEXT_PUBLIC_IS_CAP && | |
| DEVELOPER_DASHBOARD_ALLOWED_EMAILS.includes(user.email); | |
| const developerDashboardAllowedEmails = new Set(["richie@cap.so"]); | |
| const showDeveloperDashboard = | |
| buildEnv.NEXT_PUBLIC_IS_CAP && | |
| typeof user.email === "string" && | |
| developerDashboardAllowedEmails.has(user.email.toLowerCase()); |
Also: this is fine for hiding the nav item, but it shouldn’t be treated as an access control boundary (routes should still enforce permissions server-side).
1 participant
