Skip to content

JMCP: Papers - jmcp/blind-anonymous-publication-mode-Y2CwaEKp#2

Closed
landigf wants to merge 1 commit into
mainfrom
jmcp/blind-anonymous-publication-mode-Y2CwaEKp
Closed

JMCP: Papers - jmcp/blind-anonymous-publication-mode-Y2CwaEKp#2
landigf wants to merge 1 commit into
mainfrom
jmcp/blind-anonymous-publication-mode-Y2CwaEKp

Conversation

@landigf

@landigf landigf commented Mar 23, 2026

Copy link
Copy Markdown
Owner

Now I have a complete picture. Here's the review:


JMCP Review — Blind Anonymous Publication Mode

Change Summary

This PR bootstraps the entire Papers platform (72 files, ~13.4k lines) with a blind-review anonymization layer as the central feature:

  1. Contract layer (packages/contracts): Defines visibilityMode: "public" | "blind" on papers. serializePublicPaper() nulls ownerId, publicAuthorProfile, and scrubs asset filenames when mode is "blind". A runtime invariant assertBlindSafe() guards against regressions.

  2. Comment anonymization (demo-store.ts:627): getPublicComments() strips authorProfile from all comments on blind papers.

  3. AI safety gate (packages/ai): GrokProvider.assertSafe() hard-blocks any payload flagged isBlindContent or containsPrivateDraft from being sent to the external LLM.

  4. Paper creation (packages/db/src/index.ts:494): Sets publicAuthorProfile: null at write time for blind papers.

  5. UI layer (papers/[paperId]/page.tsx): Conditionally renders "Identity hidden for blind review" / "Blind participant" labels.

  6. Test coverage (contracts/test): 5 targeted tests covering profile nulling, ownerId nulling, asset filename scrubbing, leak detection, and happy path.

Validation Confidence: Medium-High

  • The core serialization + assertion pattern is sound and tested.
  • The AI gate is a hard throw — no path to accidentally send blind content.
  • Profile pages correctly filter out blind papers (visibilityMode === "public" at index.ts:463).

Remaining Risks

Risk Severity Detail
storageKey not scrubbed High serializePublicPaper scrubs fileName but leaves asset.storageKey intact (e.g. "uploads/paper_1/draft.pdf"). If storageKeys are ever exposed in URLs or API responses, they could leak the owner's identity or internal structure.
ownerId nulled only on public path Medium The internal Paper type always carries ownerId. The publicPaperSchema marks it .nullable() but serializePublicPaper for non-blind papers sets ownerId: null — good. However, the paperSchema still includes ownerId as required string, so any code path that returns a raw Paper instead of PublicPaper would leak ownership. No such path exists today, but there's no compile-time enforcement.
Comment body not scanned Medium Comments on blind papers have authorProfile stripped, but the comment body could contain self-identifying text ("As I showed in my ICML 2025 paper..."). The isBlindSafe flag is hardcoded true — no content analysis is performed.
Peer review reviewerProfile not anonymized Medium createPeerReview (index.ts:766) stores viewer.profile as reviewerProfile. For blind-mode submissions, reviewer identity is fully visible. This may be intentional (single-blind), but it's not documented.
Demo state persistence is racy Low readDemoState / writeDemoState uses read-modify-write on a JSON file with no locking. Concurrent requests can lose writes. Acceptable for demo, not for production.
slug as side-channel Low Paper slugs are derived from the title (slugify(title)-random4). Not identity-leaking, but the 4-char random suffix is weak against enumeration.

Verdict

The blind-mode serialization boundary is well-structured and the assertion guard is a good defense-in-depth pattern. The main gap is storageKey leaking through the blind serialization — that should be scrubbed alongside fileName. The reviewer identity question for peer reviews needs a product decision (single-blind vs double-blind) documented explicitly.

@landigf

landigf commented Mar 25, 2026

Copy link
Copy Markdown
Owner Author

Closing: will integrate valuable features directly on main to avoid cascade merge conflicts.

@landigf landigf closed this Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant