Conversation
… prevent counter drift
…thorized access to pending/spam comments
… access to pending or spam comments
… and prevent unauthorized comment posting
…rePost hook, enhancing security and preventing client-side ID manipulation
…ormalizing parentId, ensuring accurate cache targeting
…rify access control for comment list and count requests
…hance comment fetching logic
…anced user engagement
…oved status filters to admin sessions
…nents for enhanced user context
…re consistent dependency resolution
…aintain pending state in cache
…sistent installation and avoid cache issues
…ST response includes resolvedAuthorName and handling undefined values in getInitials function
…ies after page refresh, ensuring correct server-side handling of currentUserId
…ort in smoke tests and enhance comment thread API queries
…verification and enhance security
…r identification and pending comment visibility
… update to ensure real-time updates
…Id for e2e testing purposes across multiple examples
…comments anchor for better navigation
…ts with efficient pagination and sorting logic
…eceives complete data including resolved fields
…d, ensuring secure user identity resolution server-side
…eneration, improving code organization and reusability
…r new comments are posted
…URL for demo purposes, enhancing security awareness in examples
…and improving navigation to enhance test reliability
…hancing test flexibility and CI integration
… users and improving currentUserId handling for better user experience
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| // In production: replace with a real session/role check, e.g.: | ||
| // const session = await getSession(ctx.headers) | ||
| // if (!session?.user?.isAdmin) throw new Error("Admin access required") | ||
| console.log("onBeforeList: non-approved status filter — ensure admin check in production") |
There was a problem hiding this comment.
HIGH: No-op hook bypasses built-in 403 guard.
This hook never throws, so the built-in protection for status=pending and status=spam queries is silently disabled. Without this hook, those requests would be automatically rejected with 403. A no-op hook is worse than no hook.
Either remove onBeforeList entirely, or add a real admin check:
onBeforeList: async (query, ctx) => {
if (query.status && query.status !== 'approved') {
const session = await getSession(ctx.headers)
if (!session?.user?.isAdmin) throw new Error('Admin access required')
}
},| // request URL (NOT secure — for testing only). In a real app, derive | ||
| // the user ID from the session cookie or JWT instead. | ||
| const url = new URL((ctx.request as Request).url) | ||
| return url.searchParams.get("currentUserId") ?? null |
There was a problem hiding this comment.
HIGH: Client-controlled user identity — any caller can impersonate any user.
url.searchParams.get('currentUserId') lets any unauthenticated HTTP client supply an arbitrary user ID and receive that user's pending comments and like state. This is labeled 'NOT secure' but is committed and will be copied. Replace with session-derived identity before this merges:
resolveCurrentUserId: async (ctx) => {
const session = await getSession(ctx.headers)
return session?.user?.id ?? null
},| }, | ||
| onBeforeLike: async (commentId, authorId, ctx) => { | ||
| // In production: verify authorId matches the authenticated session | ||
| console.log("onBeforeLike: user", authorId, "toggling like on comment", commentId) |
There was a problem hiding this comment.
MEDIUM: authorId from the request body is never verified against the session.
The plugin docs warn that without this check any caller can toggle likes on behalf of arbitrary user IDs. This hook only logs — it never verifies the client-supplied authorId matches the authenticated session:
onBeforeLike: async (commentId, authorId, ctx) => {
const session = await getSession(ctx.headers)
if (!session?.user) throw new Error('Authentication required')
if (session.user.id !== authorId) throw new Error('Forbidden')
},| }, | ||
| onBeforeStatusChange: async (commentId, status, ctx) => { | ||
| // In production: verify the caller has admin/moderator role | ||
| console.log("onBeforeStatusChange: comment", commentId, "->", status) |
There was a problem hiding this comment.
MEDIUM: No-op hook — any caller can change comment status.
This hook never verifies the caller has moderator/admin privileges. onBeforeStatusChange absent → 403 by default. Hook provided but no-op → anyone can approve or spam any comment.
| }, | ||
| onBeforeDelete: async (commentId, ctx) => { | ||
| // In production: verify the caller has admin/moderator role | ||
| console.log("onBeforeDelete: comment", commentId) |
There was a problem hiding this comment.
MEDIUM: No-op hook — any caller can delete any comment.
Same pattern as onBeforeStatusChange: the hook is provided (disabling the built-in 403 guard) but never enforces any authorization check.
| const { id } = ctx.params; | ||
| const context: CommentsApiContext = { | ||
| params: ctx.params, | ||
| body: ctx.body, |
There was a problem hiding this comment.
Design observation: Unlike POST /comments (where authorId is resolved exclusively server-side via onBeforePost and is absent from the body schema), this endpoint accepts authorId from the client body and delegates verification to onBeforeLike. The examples show this delegation is easy to forget. Consider resolving the liker's identity via resolveCurrentUserId server-side — the same pattern used for POST — to eliminate the class of bugs where onBeforeLike forgets to cross-check the session.
packages/stack/src/plugins/comments/client/components/comment-form.tsx
Outdated
Show resolved
Hide resolved
…d React Router, enhancing testing capabilities across frameworks
…and accessibility
| ); | ||
| } catch (error) { | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
Redundant try-catch blocks re-throw without transformation
Low Severity
Every endpoint handler in plugin.ts wraps its logic in try { … } catch (error) { throw error; }, which is a no-op — the error propagates identically without the try-catch. This pattern appears in all seven endpoint handlers (listComments, createComment, updateComment, getCommentCount, toggleLike, updateStatus, deleteComment), adding visual noise without providing any error transformation, logging, or recovery.
Additional Locations (2)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
There are 4 total unresolved issues (including 1 from previous review).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| items: [...old.items, optimistic], | ||
| total: old.total + 1, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Optimistic reply update hides existing approved replies
Medium Severity
When posting a reply to a parent whose replies section hasn't been expanded yet, the onMutate optimistic update in usePostComment finds no existing cache (old is undefined) and creates a single-item CommentListResult. After the mutation succeeds, handleReply calls setExpandedReplies to auto-expand, causing useComments in RepliesSection to enable. It finds the single-item cache, considers it fresh (within staleTime: 30_000), and skips refetching — so existing approved replies are hidden for up to 30 seconds.
Additional Locations (1)
| ], | ||
| pageParams: [0], | ||
| }; | ||
| } |
There was a problem hiding this comment.
Hardcoded limit in optimistic infinite fallback mismatches pageSize
Low Severity
When there's no existing infinite-query cache (!old), the optimistic update creates a fallback page with limit: 10 hardcoded. The actual pageSize used by useInfiniteComments (which could be 5, 100, or any configured value) is not propagated here. This causes getNextPageParam to compute the wrong nextOffset, potentially showing or hiding the "Load more" button incorrectly until the next real fetch.
| } else { | ||
| setResolvedUserId(rawCurrentUserId ?? undefined); | ||
| } | ||
| }, [rawCurrentUserId]); |
There was a problem hiding this comment.
Duplicate currentUserId resolution logic across components
Low Severity
The currentUserId resolution logic (handling both static string and async function forms via useState + useEffect) is duplicated: inlined in resource-comments-page.tsx and extracted as useResolvedCurrentUserId in my-comments-page.internal.tsx. These are identical implementations that could share the extracted hook, reducing maintenance burden and risk of divergence.
Additional Locations (1)
1 participant
Summary
Type of change
Checklist
pnpm buildpassespnpm typecheckpassespnpm lintpassesdocs/content/docs/) if consumer-facing types or behavior changedScreenshots
Note
Medium Risk
Introduces a new Comments plugin with new HTTP endpoints and example app wiring, which affects request handling and authorization hook configuration. CI/E2E changes are low risk but may increase runtime/flakiness due to new framework-matrix execution and added smoke coverage.
Overview
Adds a first-party Comments plugin with backend routes (create/edit/status/like/delete, counts, pagination) and client UI (embeddable
CommentThread/CommentCount, moderation + “my comments” pages), exporting new entrypoints/CSS and bumping@btst/stackto2.8.0.Updates all three example apps to register the plugin on both client/server, import its CSS, and embed comment threads under blog posts and Kanban task details; also adds required
PATCHhandling to framework API catch-all routes.Expands docs/README to include the new plugin and its integration points, and overhauls E2E to add
smoke.commentscoverage plus per-framework Playwright runs (viaBTST_FRAMEWORKand a GitHub Actions matrix) with per-framework artifacts.Written by Cursor Bugbot for commit 67483fb. This will update automatically on new commits. Configure here.