fix: anonymized author ids in doubt apis#703
Conversation
|
CodeAnt AI is reviewing your PR. |
|
@madsysharma is attempting to deploy a commit to the Karan Mani Tripathi 's projects Team on Vercel. A member of the Team first needs to authorize it. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
github-actions
Bot
added
gssoc'26
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds anonymized author serialization for doubts and replies, updates API and page responses to use it, adjusts client components to render public author fields, and documents the privacy model and salt configuration. ChangesAuthor Anonymity Privacy Model
Sequence Diagram(s)sequenceDiagram
participant Client
participant DoubtsRoute as src/app/api/doubts/route.ts
participant RepliesRoute as src/app/api/replies/route.ts
participant Anonymity as src/lib/anonymity.ts
Client->>DoubtsRoute: GET /api/doubts
DoubtsRoute->>Anonymity: toPublicDoubt(doubt, email)
Anonymity-->>DoubtsRoute: publicDoubts
DoubtsRoute-->>Client: JSON response with anonymized doubts
Client->>RepliesRoute: GET /api/replies
RepliesRoute->>Anonymity: toPublicReply(reply, email)
Anonymity-->>RepliesRoute: publicReplies
RepliesRoute-->>Client: JSON response with anonymized replies
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
src/components/DoubtRepliesModal.tsx (1)
439-439: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the ownership/anonymous labels to shared constants.
This adds new inline copy for both the fallback author label and the owner suffix, and it already diverges from
DoubtCard's"You"badge. Centralizing these labels keeps the anonymous-author UI consistent and matches the TSX path rule. As per path instructions,**/*.tsxfiles should have "No hardcoded strings (use constants)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/DoubtRepliesModal.tsx` at line 439, The author display in DoubtRepliesModal is using inline fallback and owner copy, which should be centralized to match the shared anonymous/ownership labels used elsewhere. Update the reply author rendering in the DoubtRepliesModal TSX path to use shared constants for both the anonymous fallback and the owner suffix, and align the owner badge text with the existing DoubtCard convention instead of hardcoded strings.Source: Path instructions
src/__tests__/components/DoubtCard.test.tsx (1)
47-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrengthen the privacy assertion.
This only proves the anonymized handle renders. It would still pass if the card also leaked a raw email or ignored
isOwnPost. Please add a negative assertion for the email text and a positive case for the owner badge whenisOwnPostistrue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/components/DoubtCard.test.tsx` around lines 47 - 48, The DoubtCard test only checks that the anonymized handle renders, so strengthen it by adding a negative assertion that the author’s email never appears and a positive assertion that the owner badge is shown when `isOwnPost` is true. Update the existing DoubtCard test case in the `DoubtCard`/`DoubtCard.test.tsx` assertions to cover both the privacy guarantee and the ownership state.src/components/DoubtCard.tsx (1)
237-241: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the new author strings into shared constants.
Anonymousand both image alt-text templates are now duplicated inline. That makes the anonymity label easy to drift and violates the TSX rule against hardcoded strings. As per path instructions,**/*.tsxfiles should have "No hardcoded strings (use constants)".Also applies to: 331-331, 553-553
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/DoubtCard.tsx` around lines 237 - 241, The author display text and image alt-text are hardcoded in DoubtCard and duplicated across multiple TSX locations, so extract them into shared constants and reuse them in the relevant render paths. Update the DoubtCard component to reference those constants for the author fallback label and both alt-text templates, including the other duplicated spots in the same component, so the anonymity wording stays consistent and avoids inline strings in TSX.Source: Path instructions
src/__tests__/api/doubts-anonymity.test.ts (1)
78-133: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a list-route regression for author-email search.
These detail tests won’t catch the remaining
/api/doubts?search=...authorship side-channel. Add a case proving Bob/anonymous cannot find Alice’s anonymous doubt by searchingAUTHOR_EMAILor its local part.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/api/doubts-anonymity.test.ts` around lines 78 - 133, The existing doubt detail anonymity tests cover the detail endpoint but not the `/api/doubts?search=...` list route, so add a regression in the doubts anonymity test suite for search-based authorship leakage. Extend the existing setup around `makeRequest`, `currentUser`, `selectResultQueue`, and `authoredDoubt` to verify that a different signed-in user and an anonymous viewer do not get Alice’s anonymous doubt returned when searching by `AUTHOR_EMAIL` or its local part, while still confirming the author is represented only through `getAnonymousHandle`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.example:
- Around line 57-61: The current anonymity salt handling still falls back to a
public default when ANON_HANDLE_SALT is unset, which should not be allowed in
production. Update src/lib/anonymity.ts so the salt-loading logic distinguishes
production from dev/test: keep the deterministic fallback only outside
production, and fail closed in production by throwing or rejecting
startup/requests when the variable is missing. Use the existing anonymity handle
generation path and any environment check already in the module to enforce this
behavior consistently.
In `@CONTRIBUTING.md`:
- Line 242: The anonymity contract text is missing the public `authorInitial`
field even though `toPublicAuthored` and the client components rely on it.
Update the `Author anonymity` rule in CONTRIBUTING to explicitly require
`authorInitial` alongside `author` and `isOwnPost`, while still forbidding raw
doubt/reply rows or `getTableColumns(doubtsTable)` from crossing the API
boundary. This should point future routes to the contract defined by
`toPublicDoubt`, `toPublicReply`, and the updated public payload shape from
`src/lib/anonymity.ts`.
In `@src/app/api/doubts/route.ts`:
- Around line 245-248: The GET handler in route.ts still exposes author identity
through search filtering because it matches against doubtsTable.userEmail before
returning anonymized results. Update the doubts query logic in the public search
path to avoid filtering on userEmail entirely, and keep only the anonymized
fields returned by toPublicDoubt; if author-specific lookup is needed, handle it
separately in an authenticated/private path using the existing route handlers
and symbols like toPublicDoubt, doubtsTable, and the GET search builder.
In `@src/components/DoubtCard.tsx`:
- Around line 84-90: The ownership sync in DoubtCard’s useEffect only ever sets
state to true, so when doubt.isOwnPost changes back to false the component can
keep stale owner-only UI. Update the effect to fully mirror doubt.isOwnPost in
the isOwner state, or derive isOwner directly from the prop, and make sure the
logic in DoubtCard stays correct when the card is reused for different posts.
In `@src/components/DoubtRepliesModal.tsx`:
- Around line 71-75: The DoubtRepliesModal effect only ever sets isDoubtOwner to
true, so the previous thread’s owner state can leak when switching doubts.
Update the useEffect in DoubtRepliesModal to reset isDoubtOwner based on the
current doubt.isOwnPost value whenever the modal/thread changes (including when
it becomes false), and keep the dependency list aligned with the state it
derives from.
In `@src/lib/anonymity.ts`:
- Around line 27-30: handleSalt currently falls back to DEFAULT_HANDLE_SALT
whenever ANON_HANDLE_SALT is missing, which should not happen in production.
Update handleSalt in anonymity.ts so that it fails closed in production by
throwing or otherwise refusing to continue when the env var is absent, while
preserving the current fallback behavior only for non-production environments.
Use the existing handleSalt symbol and the environment check already available
in this module to keep the fix localized.
- Around line 84-118: The serializer chokepoint in toPublicAuthored() only
strips userEmail, embedding, and deletedAt, so any joined aliases like userId,
authorId, clerkId, email, or name can still leak identity. Update
AuthoredRow/PublicAuthored and the destructuring in toPublicAuthored() to remove
all author-identifying aliases before returning the safe object, while still
preserving non-identity content fields and deriving author, authorInitial, and
isOwnPost from the viewer/session data.
In `@src/types/index.ts`:
- Around line 221-226: The `Doubt` and `Reply` types in `src/types/index.ts`
currently blur raw DB rows and sanitized API payloads by still allowing
`userEmail` while leaving `author`, `authorInitial`, and `isOwnPost` optional.
Split these into separate raw and public authored shapes, or add a
`PublicAuthored<T>` alias, so the public API type explicitly omits `userEmail`
and requires the anonymized fields. Update the `Doubt`/`Reply` definitions and
any related authored interfaces to use the new public type, keeping raw row
types distinct so TypeScript catches privacy regressions.
---
Nitpick comments:
In `@src/__tests__/api/doubts-anonymity.test.ts`:
- Around line 78-133: The existing doubt detail anonymity tests cover the detail
endpoint but not the `/api/doubts?search=...` list route, so add a regression in
the doubts anonymity test suite for search-based authorship leakage. Extend the
existing setup around `makeRequest`, `currentUser`, `selectResultQueue`, and
`authoredDoubt` to verify that a different signed-in user and an anonymous
viewer do not get Alice’s anonymous doubt returned when searching by
`AUTHOR_EMAIL` or its local part, while still confirming the author is
represented only through `getAnonymousHandle`.
In `@src/__tests__/components/DoubtCard.test.tsx`:
- Around line 47-48: The DoubtCard test only checks that the anonymized handle
renders, so strengthen it by adding a negative assertion that the author’s email
never appears and a positive assertion that the owner badge is shown when
`isOwnPost` is true. Update the existing DoubtCard test case in the
`DoubtCard`/`DoubtCard.test.tsx` assertions to cover both the privacy guarantee
and the ownership state.
In `@src/components/DoubtCard.tsx`:
- Around line 237-241: The author display text and image alt-text are hardcoded
in DoubtCard and duplicated across multiple TSX locations, so extract them into
shared constants and reuse them in the relevant render paths. Update the
DoubtCard component to reference those constants for the author fallback label
and both alt-text templates, including the other duplicated spots in the same
component, so the anonymity wording stays consistent and avoids inline strings
in TSX.
In `@src/components/DoubtRepliesModal.tsx`:
- Line 439: The author display in DoubtRepliesModal is using inline fallback and
owner copy, which should be centralized to match the shared anonymous/ownership
labels used elsewhere. Update the reply author rendering in the
DoubtRepliesModal TSX path to use shared constants for both the anonymous
fallback and the owner suffix, and align the owner badge text with the existing
DoubtCard convention instead of hardcoded strings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4597a8e6-8059-4858-81fc-b7404b8982ac
📒 Files selected for processing (15)
.env.exampleCONTRIBUTING.mdREADME.mddocs/PRIVACY.mdsrc/__tests__/api/doubts-anonymity.test.tssrc/__tests__/components/DoubtCard.test.tsxsrc/__tests__/lib/anonymity.test.tssrc/app/api/doubts/[id]/route.tssrc/app/api/doubts/route.tssrc/app/api/replies/route.tssrc/app/doubts/[id]/page.tsxsrc/components/DoubtCard.tsxsrc/components/DoubtRepliesModal.tsxsrc/lib/anonymity.tssrc/types/index.ts
knoxiboy
added
level:advanced
knoxiboy
left a comment
knoxiboy
left a comment
There was a problem hiding this comment.
Implement suggestions from bots to improve the pr
|
CodeAnt AI is running Incremental review |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI Incremental review completed. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/api/replies/route.ts (1)
89-100: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winMap
dataand remove the straycatch/return; this route is syntactically broken. The handler buildsdata, but the response mapsrepliesWithVotes, and the extracatch/raw return before the finalcatchleaves invalid control flow insrc/app/api/replies/route.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/replies/route.ts` around lines 89 - 100, The replies route has broken control flow and a data mismatch: the handler should return the mapped `data`/public replies, but it currently returns `repliesWithVotes` and includes a stray `return` plus duplicate `catch` block. In `src/app/api/replies/route.ts`, fix the `GET` handler around `toPublicReply`/`buildErrorResponse` so there is only one `try/catch`, remove the extra unreachable `return`, and ensure the JSON response uses the anonymized mapped result instead of the raw replies.Source: Linters/SAST tools
src/app/api/doubts/route.ts (1)
436-445: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winMove the create response out of the tag loop and remove the stray fallback branch. The current return still sits inside the
forloop, so tagged requests can exit early, and the extracatch/rawnewDoubtpath leaves the handler malformed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/doubts/route.ts` around lines 436 - 445, The create response in the doubts route is still nested in the tag-processing flow and there is a leftover fallback branch that makes the handler malformed. Move the NextResponse.json(toPublicDoubt({ ...newDoubt, tags: savedTags }, email)) return so it runs after the tag loop in the create handler, and remove the stray raw return/catch path that follows it. Use the create handler logic around newDoubt, savedTags, and buildErrorResponse to keep only one valid success path and one error path.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/api/doubts/route.ts`:
- Around line 259-288: The doubts endpoint is bypassing the intended flow: GET
should return the anonymized public payload, and POST is returning early before
rate limiting, validation, and create logic. Update the route in
`src/app/api/doubts/route.ts` so `GET` uses the public serializer
(`toPublicDoubt` / `publicDoubts`) before responding, and remove the premature
return from `POST` so it continues into the existing create path after
`currentUser()` and request handling.
In `@src/app/api/replies/route.ts`:
- Around line 270-277: The reply creation handler in the route for replies has a
broken control-flow structure and an unsafe fallback response. Fix the
`createReply`/reply-create logic by properly closing the `if` and `try` blocks
so the code parses correctly, and ensure every successful return uses
`toPublicReply(newReply[0], email)` instead of returning the raw reply object.
Keep the error handling via `buildErrorResponse` intact and remove the stray
`return NextResponse.json(newReply[0])` path so no response leaks `userEmail`.
---
Outside diff comments:
In `@src/app/api/doubts/route.ts`:
- Around line 436-445: The create response in the doubts route is still nested
in the tag-processing flow and there is a leftover fallback branch that makes
the handler malformed. Move the NextResponse.json(toPublicDoubt({ ...newDoubt,
tags: savedTags }, email)) return so it runs after the tag loop in the create
handler, and remove the stray raw return/catch path that follows it. Use the
create handler logic around newDoubt, savedTags, and buildErrorResponse to keep
only one valid success path and one error path.
In `@src/app/api/replies/route.ts`:
- Around line 89-100: The replies route has broken control flow and a data
mismatch: the handler should return the mapped `data`/public replies, but it
currently returns `repliesWithVotes` and includes a stray `return` plus
duplicate `catch` block. In `src/app/api/replies/route.ts`, fix the `GET`
handler around `toPublicReply`/`buildErrorResponse` so there is only one
`try/catch`, remove the extra unreachable `return`, and ensure the JSON response
uses the anonymized mapped result instead of the raw replies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3d116ad8-aca9-4cfc-b70c-933aa2aeb401
📒 Files selected for processing (3)
src/app/api/doubts/[id]/route.tssrc/app/api/doubts/route.tssrc/app/api/replies/route.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/api/doubts/[id]/route.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/api/replies/route.ts (1)
89-100: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winMap
dataand remove the straycatch/return; this route is syntactically broken. The handler buildsdata, but the response mapsrepliesWithVotes, and the extracatch/raw return before the finalcatchleaves invalid control flow insrc/app/api/replies/route.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/replies/route.ts` around lines 89 - 100, The replies route has broken control flow and a data mismatch: the handler should return the mapped `data`/public replies, but it currently returns `repliesWithVotes` and includes a stray `return` plus duplicate `catch` block. In `src/app/api/replies/route.ts`, fix the `GET` handler around `toPublicReply`/`buildErrorResponse` so there is only one `try/catch`, remove the extra unreachable `return`, and ensure the JSON response uses the anonymized mapped result instead of the raw replies.Source: Linters/SAST tools
src/app/api/doubts/route.ts (1)
436-445: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winMove the create response out of the tag loop and remove the stray fallback branch. The current return still sits inside the
forloop, so tagged requests can exit early, and the extracatch/rawnewDoubtpath leaves the handler malformed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/doubts/route.ts` around lines 436 - 445, The create response in the doubts route is still nested in the tag-processing flow and there is a leftover fallback branch that makes the handler malformed. Move the NextResponse.json(toPublicDoubt({ ...newDoubt, tags: savedTags }, email)) return so it runs after the tag loop in the create handler, and remove the stray raw return/catch path that follows it. Use the create handler logic around newDoubt, savedTags, and buildErrorResponse to keep only one valid success path and one error path.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/api/doubts/route.ts`:
- Around line 259-288: The doubts endpoint is bypassing the intended flow: GET
should return the anonymized public payload, and POST is returning early before
rate limiting, validation, and create logic. Update the route in
`src/app/api/doubts/route.ts` so `GET` uses the public serializer
(`toPublicDoubt` / `publicDoubts`) before responding, and remove the premature
return from `POST` so it continues into the existing create path after
`currentUser()` and request handling.
In `@src/app/api/replies/route.ts`:
- Around line 270-277: The reply creation handler in the route for replies has a
broken control-flow structure and an unsafe fallback response. Fix the
`createReply`/reply-create logic by properly closing the `if` and `try` blocks
so the code parses correctly, and ensure every successful return uses
`toPublicReply(newReply[0], email)` instead of returning the raw reply object.
Keep the error handling via `buildErrorResponse` intact and remove the stray
`return NextResponse.json(newReply[0])` path so no response leaks `userEmail`.
---
Outside diff comments:
In `@src/app/api/doubts/route.ts`:
- Around line 436-445: The create response in the doubts route is still nested
in the tag-processing flow and there is a leftover fallback branch that makes
the handler malformed. Move the NextResponse.json(toPublicDoubt({ ...newDoubt,
tags: savedTags }, email)) return so it runs after the tag loop in the create
handler, and remove the stray raw return/catch path that follows it. Use the
create handler logic around newDoubt, savedTags, and buildErrorResponse to keep
only one valid success path and one error path.
In `@src/app/api/replies/route.ts`:
- Around line 89-100: The replies route has broken control flow and a data
mismatch: the handler should return the mapped `data`/public replies, but it
currently returns `repliesWithVotes` and includes a stray `return` plus
duplicate `catch` block. In `src/app/api/replies/route.ts`, fix the `GET`
handler around `toPublicReply`/`buildErrorResponse` so there is only one
`try/catch`, remove the extra unreachable `return`, and ensure the JSON response
uses the anonymized mapped result instead of the raw replies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3d116ad8-aca9-4cfc-b70c-933aa2aeb401
📒 Files selected for processing (3)
src/app/api/doubts/[id]/route.tssrc/app/api/doubts/route.tssrc/app/api/replies/route.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/api/doubts/[id]/route.ts
🛑 Comments failed to post (2)
src/app/api/doubts/route.ts (1)
259-288: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Verify the serializer is in GET and no GET pagination variables are referenced at the top of POST. sed -n '250,300p' src/app/api/doubts/route.tsRepository: knoxiboy/DoubtDesk
Length of output: 1566
🏁 Script executed:
#!/bin/bash nl -ba src/app/api/doubts/route.ts | sed -n '245,320p'Repository: knoxiboy/DoubtDesk
Length of output: 192
🏁 Script executed:
#!/bin/bash sed -n '245,320p' src/app/api/doubts/route.ts | cat -nRepository: knoxiboy/DoubtDesk
Length of output: 2855
Restore the public serializer in
GETand remove the earlyPOSTreturn.
GETstill returns rawdoubts, andPOSTexits before rate limiting, validation, and create logic, so the endpoint never creates a doubt.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/doubts/route.ts` around lines 259 - 288, The doubts endpoint is bypassing the intended flow: GET should return the anonymized public payload, and POST is returning early before rate limiting, validation, and create logic. Update the route in `src/app/api/doubts/route.ts` so `GET` uses the public serializer (`toPublicDoubt` / `publicDoubts`) before responding, and remove the premature return from `POST` so it continues into the existing create path after `currentUser()` and request handling.src/app/api/replies/route.ts (1)
270-277: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Inspect POST response construction and development-branch braces. sed -n '250,280p' src/app/api/replies/route.tsRepository: knoxiboy/DoubtDesk
Length of output: 1459
🏁 Script executed:
#!/bin/bash # Inspect the full reply route around the POST handler to understand control flow and response shaping. sed -n '1,340p' src/app/api/replies/route.tsRepository: knoxiboy/DoubtDesk
Length of output: 9862
🏁 Script executed:
#!/bin/bash # Show exact line numbers around the POST handler tail to resolve the brace and return structure. nl -ba src/app/api/replies/route.ts | sed -n '240,290p'Repository: knoxiboy/DoubtDesk
Length of output: 192
🏁 Script executed:
#!/bin/bash # Get numbered lines with a plain-text fallback to inspect the tail of the POST handler. python3 - <<'PY' from pathlib import Path p = Path('src/app/api/replies/route.ts') for i, line in enumerate(p.read_text().splitlines(), 1): if 245 <= i <= 290: print(f"{i:4d}: {line}") PYRepository: knoxiboy/DoubtDesk
Length of output: 1807
Fix the reply-create response path. The block here doesn’t parse as written, and the non-development branch still returns raw
newReply[0]withuserEmail; close theif/trystructure and returntoPublicReply(newReply[0], email)on every path.🧰 Tools
🪛 Biome (2.5.0)
[error] 277-277: Expected a statement but instead found 'catch (error)'.
(parse)
🪛 GitHub Check: TypeScript Check
[failure] 277-277:
'try' expected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/replies/route.ts` around lines 270 - 277, The reply creation handler in the route for replies has a broken control-flow structure and an unsafe fallback response. Fix the `createReply`/reply-create logic by properly closing the `if` and `try` blocks so the code parses correctly, and ensure every successful return uses `toPublicReply(newReply[0], email)` instead of returning the raw reply object. Keep the error handling via `buildErrorResponse` intact and remove the stray `return NextResponse.json(newReply[0])` path so no response leaks `userEmail`.Source: Linters/SAST tools
2a49d44 to
d08dc4e
Compare
|
Hi @knoxiboy , have included the suggested changes. Please review this PR. (NOTE: the build and TypeScript errors shown were already present prior to my changes). Thanks. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/offline/syncQueue.ts (1)
171-180: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReturn the full public-author shape for pending items.
Line 173 and Line 191 only rename
userEmailtoauthor, but the offline payloads still miss the fields the UI now relies on.DoubtCardusesauthorInitialandisOwnPost, andDoubtRepliesModalusesisOwnPostfor right-side alignment and the(YOU)label. As written, the current user's own pending doubts/replies render like someone else's post.Suggested fix
export async function getPendingDoubts(): Promise<any[]> { const queue = await getQueue(); return queue .filter(item => item.url === "/api/doubts") .map(item => ({ id: `pending-${item.id}`, author: "Pending User", + authorInitial: "P", subject: item.payload.subject || "Subject", content: item.payload.content || "", imageUrl: item.payload.imageUrl || "", tags: (item.payload.tags || []).map((t: string) => ({ name: t })), isSolved: "unsolved", createdAt: item.payload.createdAt || new Date(item.timestamp).toISOString(), + isOwnPost: true, isPendingSync: true })); } export async function getPendingReplies(doubtId: number): Promise<any[]> { const queue = await getQueue(); return queue .filter(item => item.url === "/api/replies" && Number(item.payload.doubtId) === Number(doubtId)) .map(item => ({ id: `pending-${item.id}`, doubtId, author: "Pending User", type: item.payload.type || "comment", content: item.payload.content || "", imageUrl: item.payload.imageUrl || "", upvotes: 0, createdAt: item.payload.createdAt || new Date(item.timestamp).toISOString(), + isOwnPost: true, isPendingSync: true })); }Also applies to: 188-197
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/offline/syncQueue.ts` around lines 171 - 180, The pending-item mapping in syncQueue.ts is only renaming userEmail to author, but it still omits UI-dependent public-author fields. Update the pending doubt/reply objects created in the offline queue mapping to include the full shape expected by DoubtCard and DoubtRepliesModal, especially authorInitial and isOwnPost alongside author. Use the existing pending item data and current user identity to compute those fields so the user’s own pending posts render with the correct alignment and “(YOU)” label.
🧹 Nitpick comments (1)
src/components/DoubtCard.tsx (1)
235-239: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the anonymous fallback into a shared constant.
These changed render paths now inline the same
"Anonymous"label in three places, which makes the visible name and image alt text easy to drift.Proposed fix
+const ANONYMOUS_AUTHOR_LABEL = "Anonymous"; + export default function DoubtCard({ doubt, onUpdate, onViewAISolution, role, openRepliesOnMount = false, disableModal = false, onCommentClick }: DoubtCardProps) { + const authorLabel = doubt.author || ANONYMOUS_AUTHOR_LABEL; @@ - {doubt.author || 'Anonymous'} + {authorLabel} @@ - alt={`Doubt image for ${doubt.subject} by ${doubt.author || 'Anonymous'}`} + alt={`Doubt image for ${doubt.subject} by ${authorLabel}`} @@ - alt={`Full view of doubt image for ${doubt.subject} by ${doubt.author || 'Anonymous'}`} + alt={`Full view of doubt image for ${doubt.subject} by ${authorLabel}`}As per path instructions,
**/*.tsx: Review for: - No hardcoded strings (use constants).Also applies to: 329-329, 551-551
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/DoubtCard.tsx` around lines 235 - 239, The DoubtCard render paths are hardcoding the same anonymous fallback label in multiple places, so extract that value into a shared constant and reuse it for the visible name and any related alt text. Update the affected JSX in DoubtCard so the fallback for doubt.author and the other matching render paths all reference the same constant, keeping the anonymous label consistent across the component.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/lib/anonymity.test.ts`:
- Around line 54-57: Restore NODE_ENV to its original unset state in the
anonymity test cleanup: the afterEach block currently assigns
mutableEnv.NODE_ENV = origEnv, which leaves NODE_ENV defined when origEnv was
undefined. Update the cleanup logic in the afterEach callback so it deletes
NODE_ENV when the saved original value is undefined, and only reassigns it when
a real value exists; keep the ANON_HANDLE_SALT restore logic alongside it.
---
Outside diff comments:
In `@src/lib/offline/syncQueue.ts`:
- Around line 171-180: The pending-item mapping in syncQueue.ts is only renaming
userEmail to author, but it still omits UI-dependent public-author fields.
Update the pending doubt/reply objects created in the offline queue mapping to
include the full shape expected by DoubtCard and DoubtRepliesModal, especially
authorInitial and isOwnPost alongside author. Use the existing pending item data
and current user identity to compute those fields so the user’s own pending
posts render with the correct alignment and “(YOU)” label.
---
Nitpick comments:
In `@src/components/DoubtCard.tsx`:
- Around line 235-239: The DoubtCard render paths are hardcoding the same
anonymous fallback label in multiple places, so extract that value into a shared
constant and reuse it for the visible name and any related alt text. Update the
affected JSX in DoubtCard so the fallback for doubt.author and the other
matching render paths all reference the same constant, keeping the anonymous
label consistent across the component.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 70519d49-1ff4-4a3b-bc93-9f9d13e64d79
📒 Files selected for processing (17)
.env.exampleCONTRIBUTING.mdREADME.mddocs/PRIVACY.mdsrc/__tests__/api/doubts-anonymity.test.tssrc/__tests__/components/DoubtCard.test.tsxsrc/__tests__/lib/anonymity.test.tssrc/app/api/doubts/[id]/route.tssrc/app/api/doubts/route.tssrc/app/api/replies/route.tssrc/app/doubts/[id]/page.tsxsrc/components/DoubtCard.tsxsrc/components/DoubtRepliesModal.tsxsrc/components/InfiniteDoubtFeed.tsxsrc/lib/anonymity.tssrc/lib/offline/syncQueue.tssrc/types/index.ts
✅ Files skipped from review due to trivial changes (4)
- .env.example
- CONTRIBUTING.md
- docs/PRIVACY.md
- README.md
🚧 Files skipped from review as they are similar to previous changes (8)
- src/app/api/doubts/[id]/route.ts
- src/tests/components/DoubtCard.test.tsx
- src/app/doubts/[id]/page.tsx
- src/tests/api/doubts-anonymity.test.ts
- src/app/api/replies/route.ts
- src/components/DoubtRepliesModal.tsx
- src/lib/anonymity.ts
- src/types/index.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
github-actions
Bot
added
gssoc:approved
2 participants
User description
Description
Stops the doubt and reply APIs from exposing a post author's real identity to other users. Anonymous doubts/replies now return only an anonymized, non-reversible handle plus a server-computed
isOwnPostflag; the author'suserEmail(and internal fields like the embedding vector) never leave the server.Related Issue
Closes #657
Type of Change
Problem
Both
GET /api/doubtsandGET /api/doubts/[id]selected...getTableColumns(doubtsTable)and returned the row verbatim, andGET /api/repliesreturned full reply rows. That payload includesdoubts.userEmail/replies.userEmail: the author's real email. The UI showed a "Student" style label, but the email was sitting in the JSON (and was even rendered directly:DoubtCarddisplayeduserEmail.split('@')[0]). The server-rendered permalink page also passed the full row into a client component, re-leaking it through the RSC payload.Result: any user (in some paths, even unauthenticated) could read another user's email from the network response despite the "anonymous" presentation: which is an Excessive Data Exposure / Broken Object Property-Level auth issue.
Root cause
No serialization boundary between the DB row and the API response for doubts/replies, so internal/identifying columns flowed straight to clients (the classroom-members endpoint,
GET /api/rooms/members, already does this correctly: privileged roles see emails, everyone else gets aStudent_<id>handle. This PR brings doubts/replies up to that same standard.)Fix
A single serialization chokepoint, then route every author-facing response through it.
src/lib/anonymity.ts(new file)getAnonymousHandle(email)-> deterministic, salted, non-reversibleStudent_XXXXXhandle. It isbase36(first 40 bits of SHA-256(salt + ":" + email)), salted withANON_HANDLE_SALTso handles can't be pre-computed from candidate emails. Deterministic so an author reads consistently within a thread; returns"Anonymous"when there is no author.getAnonymousInitial(email)-> avatar letter derived from the handle (never the email).toPublicDoubt/toPublicReply(aliases oftoPublicAuthored) -> stripuserEmail,embedding, anddeletedAt; attachauthor,authorInitial, andisOwnPost(computed server-side from the session). All other fields (content, tags, likes,hasLiked,replyCount,hasUpvoted, etc.) pass through untouched.Serialized routes
GET /api/doubts(list)GET /api/doubts/[id](detail)POST /api/doubts(create: author's own; still sanitized,isOwnPost: true)GET /api/replies(list)POST /api/replies(create: author's own; still sanitized)app/doubts/[id]/page.tsx: sanitizes the row before handing it to the client component (RSC props are client-visible). Server-side authorization checks above still use the raw row.Frontend changes
DoubtCardandDoubtRepliesModalnow render theauthorhandle /authorInitialand use the serverisOwnPostflag for ownership controls and the "(YOU)" indicator, instead of comparing raw emails on the client.DoubtandReplytypes gainauthor?,authorInitial?,isOwnPost?.Controlled exceptions
The real identity stays available server-side for legitimate use:
isOwnPost: true)moderation_logstable and admin views: a privileged, authenticated surface, untouched here)See
docs/PRIVACY.mdfor the full model.Testing
src/__tests__/lib/anonymity.test.ts: handle is deterministic, salted, formatted, case-insensitive, and never contains the email or its local-part; serializer stripsuserEmail/embedding/deletedAt, setsisOwnPostcorrectly, preserves other fields, and never serializes the author email anywhere in the output.src/__tests__/api/doubts-anonymity.test.ts: User A authors an anonymous community doubt; User B and an anonymous viewer fetch it and receive the handle +isOwnPost: falsewith nouserEmail/email/embeddingin the response; the author getsisOwnPost: truebut still no raw email.src/__tests__/components/DoubtCard.test.tsxto assert the handle is rendered (not the email local-part).npx tsc --noEmitclean;npm testgreen (36 suites / 185 tests);eslintclean on changed files.Edge cases
isOwnPostisfalse, handle still shown, no identifiers leak.isOwnPost: trueconveys ownership without an email).embeddingfrom responses also removes a large internal vector from every doubt payload (a payload-size win).docs/PRIVACY.md.Documentation
docs/PRIVACY.md: new "Privacy Model" doc (guarantee, exposed vs. hidden fields, handle derivation, enforcement points,ANON_HANDLE_SALT).README.md: "Privacy Model" section under Features.CONTRIBUTING.md: API Response Contract rule: doubt/reply responses must go throughtoPublicDoubt/toPublicReply; never return raw rows..env.example: documentsANON_HANDLE_SALT.Checklist
tsc,test, andeslintpassany)Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
ANON_HANDLE_SALTconfiguration expectations.Tests
CodeAnt-AI Description
Keep doubt and reply authors anonymous in all client-facing views
What Changed
isOwnPostvalue, instead of the author's emailisOwnPostto mark the current user's own postsImpact
✅ Fewer email leaks in doubts and replies✅ Safer anonymous posting✅ Clearer own-post markers for students💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.