fix(posts): guard merge visibility leaks#31
Open
BunsDev wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens merge-related visibility handling to prevent public endpoints from accidentally exposing private/internal-board content when posts are merged across boards with different isPublic settings.
Changes:
- Add a merge-time validation in
mergePostthat rejects merges when the two posts’ boards differ inisPublic(newINCOMPATIBLE_MERGE_VISIBILITYerror). - Add public-safe merge helpers (
getPublicMergedPosts,getPublicPostMergeInfo) and update portal post-detail fetching to use them. - Restrict the public post-detail comment query to include merged child posts only when the child post’s board is public; add a regression unit test for cross-visibility merge attempts.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/web/src/lib/server/functions/portal.ts | Switch public post-detail merge metadata fetching to public-safe merge helpers. |
| apps/web/src/lib/server/domains/posts/post.public.detail.ts | Filter merged-child comment inclusion to only child posts on public boards to prevent comment leakage. |
| apps/web/src/lib/server/domains/posts/post.merge.ts | Enforce same-visibility merges; introduce public-safe merge info/list helpers for portal use. |
| apps/web/src/lib/server/domains/posts/tests/post-merge.test.ts | Update mocks for isPublic and add regression coverage for cross-visibility merge rejection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0754fc0 to
51cfc28
Compare
51cfc28 to
84529d7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Description
mergePostvalidate both posts'board.isPublicby loading both boards and reject merges whenisPublicdiffers, returningINCOMPATIBLE_MERGE_VISIBILITYand avoiding writingcanonicalPostId.getPublicMergedPostsandgetPublicPostMergeInfothat only return merged posts and canonical metadata when related posts/boards are public, and update portal handlers to use them instead of the unrestricted helpers.isPublicvalues and update existing mocks to account forisPublic.post.merge.ts,post.public.detail.ts,portal.ts, and thepost-mergeunit test.Testing
bunx prettier --checkon the modified files succeeded.bun run typecheckcould not complete in this environment due to missing dev type packages (@testing-library/jest-dom,buntypes) because dependencies were not installed.bun testproduced an environment mismatch (vi.mockunavailable) andbun run testfailed because thevitestexecutable is not present, andbun install --frozen-lockfilewas blocked by registry 403 errors, preventing full test runs here.Codex Task