Repository navigation
chore(sync): merge upstream documenso/main (c81bc72c4..8a41a3bf6, incl. v2.19.0) - #23
Merged
Merged
Conversation
Strip encryption from PDFs that open with an empty user password via libpdf's ignorePermissions, still rejecting user-password PDFs. Upgrade @libpdf/core to 0.5.1, which also keeps overlapping and layered text intact during text extraction. Resolves documenso#3303
Send completed/rejected events when reopening an actioned v1 embed, show the completed page after signing in v2, and tidy the completed page.
# Conflicts: # apps/remix/app/components/forms/signup.tsx
|
Important Review skippedToo many files! This PR contains 170 files, which is 70 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (170)
You can disable this status message by setting the
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 |
github-actions
Bot
added
apps: web
type: documentation
version bump 👀
🚨 e2e changes 🚨
labels
![dos[bot]](https://avatars.githubusercontent.com/in/4711427?s=60&v=4){kind=small}
There was a problem hiding this comment.
⏱️ Code Review completed (31 files · 720,706 chars · 3 PR unit(s))
ℹ️ Full-Context Analysis: Analyzed all changed files in a unified context pass to preserve cross-file type definitions, imports, and caller contracts. Deducted 3 PR units.
⚠️ PR diff exceeded maximum review ceiling (3 batches / ~90,000 chars) - lower-priority files were skipped.
⏱️ Adversarial Review completed (Model: gemini-3.8-flash)
⚠️ Primary model was unavailable; the verdict was served by backup modelgemini-3.8-flash.
⚠️ Phase 1 reviewer output hit the token limit; the candidate list may be incomplete.
🔍 Verified Adversarial Review Findings
An issue identified in the Phase 1 review is verified:
📋 Findings Summary (1 inline finding)
- 🔴 BLOCKER
packages/lib/client-only/hooks/use-editor-recipients.ts:70-86: Unconditional rejection of recipient signing groups across all envelopes in the editor (Inline on diff)
Comment on lines
+70
to
+86
| const seenSigningOrders = new Set<number>(); | ||
|
|
||
| data.signers.forEach((signer, index) => { | ||
| if (signer.role === RecipientRole.CC || typeof signer.signingOrder !== 'number') { | ||
| return; | ||
| } | ||
|
|
||
| if (seenSigningOrders.has(signer.signingOrder)) { | ||
| ctx.addIssue({ | ||
| code: z.ZodIssueCode.custom, | ||
| message: 'CSC envelopes do not support recipient signing groups.', | ||
| path: ['signers', index, 'signingOrder'], | ||
| }); | ||
| } | ||
|
|
||
| seenSigningOrders.add(signer.signingOrder); | ||
| }); |
There was a problem hiding this comment.
🔴 BLOCKER: Unconditional rejection of recipient signing groups across all envelopes in the editor
Failure Trace:
- A user creates or edits a
⚠️ Verdict output hit the token limit and may be truncated.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive analytics dashboard for teams and organisations, refactors recipient signing order and grouping logic to support signing groups while enforcing sequential signing for TSP envelopes, adds public legal links, and improves PDF password-protection handling. Feedback from the review highlights a high-severity security issue where sensitive signing tokens are exposed via postMessage with a wildcard target origin ('*'), alongside a React hook anti-pattern in useEffect that creates stale closures.
Comment on lines
+270
to
+306
| useEffect(() => { | ||
| if (!window.parent) { | ||
| return; | ||
| } | ||
|
|
||
| if (hasRejectedDocument) { | ||
| window.parent.postMessage( | ||
| { | ||
| action: 'document-rejected', | ||
| data: { | ||
| token, | ||
| documentId, | ||
| recipientId: recipient.id, | ||
| }, | ||
| }, | ||
| '*', | ||
| ); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| if (hasCompletedDocument) { | ||
| window.parent.postMessage( | ||
| { | ||
| action: 'document-completed', | ||
| data: { | ||
| token, | ||
| documentId, | ||
| recipientId: recipient.id, | ||
| }, | ||
| }, | ||
| '*', | ||
| ); | ||
| } | ||
|
|
||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, []); |
There was a problem hiding this comment.
Security & Code Quality Issues\n\n1. Sensitive Data Exposure (High Severity): Sending the sensitive signing token via postMessage with a wildcard target origin ('*') allows any parent window (including malicious or hijacked frames) to intercept the token. Consider restricting the target origin to a trusted origin (e.g., a configured parent origin) instead of using * when transmitting sensitive credentials.\n\n2. React Hook Anti-Pattern (Medium Severity): This useEffect uses an empty dependency array [] but references state variables hasRejectedDocument and hasCompletedDocument. This creates a stale closure and requires disabling the react-hooks/exhaustive-deps lint rule. Since this effect is intended to notify the parent window on mount if the document is already completed or rejected, you should use the initial props isRejected and isCompleted directly and include them in the dependency array.
useEffect(() => {\n if (!window.parent) {\n return;\n }\n\n if (isRejected) {\n window.parent.postMessage(\n {\n action: 'document-rejected',\n data: {\n token,\n documentId,\n recipientId: recipient.id,\n },\n },\n '*',\n );\n\n return;\n }\n\n if (isCompleted) {\n window.parent.postMessage(\n {\n action: 'document-completed',\n data: {\n token,\n documentId,\n recipientId: recipient.id,\n },\n },\n '*',\n );\n }\n }, [isRejected, isCompleted, token, documentId, recipient.id]);
Upstream documenso#3131 rewrote the region where our fork adds the oidcProviderLabel prop (used for the OIDC signup button label in redirect-only mode), and the auto-merge silently dropped it while the route still passed it - a semantic conflict the textual merge could not see.
Without NEXT_PUBLIC_TERMS_OF_SERVICE_URL / NEXT_PUBLIC_PRIVACY_POLICY_URL set, the signup consent text and the new public legal footers render nothing on self-host. Defaults point at the local /terms and /privacy pages added in PR #18.
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
8 participants
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.
What
Upstream sync #2: merges documenso/documenso upstream/main (17 commits since our last sync point c81bc72), spanning the v2.19.0 release (586b1f5) plus 8 post-release commits.
Highlights: configurable legal/data-protection/imprint links (documenso#3131 - complements our /terms + /privacy pages), recipient grouping, team document analytics dashboard, owner-password-protected PDF acceptance, embed signing completion fixes, signed-recipient reject prevention, branding form validation errors.
Conflict resolution (1 file)
apps/remix/app/components/forms/signup.tsx: took upstream's configurableavailableLegalLinksmechanism (envNEXT_PUBLIC_TERMS_OF_SERVICE_URL/NEXT_PUBLIC_PRIVACY_POLICY_URL/NEXT_PUBLIC_IMPRINT_URL, newpublic-legal-linkscomponent also renders on recipient signing pages) over our hardcoded /terms /privacy links from PR feat(branding): replace the last user-facing Documenso surfaces (batch 1) #18. Deploy note: prod env must set the two legal URLs tohttps://sign.crove.com/termsandhttps://sign.crove.com/privacyso the links render (self-host default renders NO legal links when unset).Plus a format-only commit conforming upstream's
app-command-menu.tsxto our biome config.Verification
Deploy note
Set on prod before/with deploy:
NEXT_PUBLIC_TERMS_OF_SERVICE_URL="https://sign.crove.com/terms"andNEXT_PUBLIC_PRIVACY_POLICY_URL="https://sign.crove.com/privacy"(self-host default renders NO legal links when unset).📌 TL;DR
This PR introduces a comprehensive analytics dashboard for teams and organizations, featuring new UI components for data visualization and activity tracking. It also enhances the embedded signing experience with new CSS hooks and postMessage events for completion/rejection states, while improving form validation and legal link configuration.
🎯 Type of Change
🔍 Changes Walkthrough
apps/docs/content/docs/developers/embedding/css-variables.mdxdata-readonlyattribute. Fixed CSS variable usage in examples.apps/docs/content/docs/self-hosting/configuration/database.mdxnpx prisma migrate deploycommand from manual deployment instructions.apps/docs/content/docs/self-hosting/maintenance/upgrades.mdxNEXT_PRIVATE_DIRECT_DATABASE_URLand explicit schema path.apps/remix/app/components/dialogs/envelope-item-edit-dialog.tsxapps/remix/app/components/embed/embed-document-completed.tsxapps/remix/app/components/embed/embed-document-signing-page-v1.tsxisRejectedprop and implementedpostMessageevents to the parent window fordocument-rejectedanddocument-completedactions.apps/remix/app/components/embed/embed-document-signing-page-v2.tsxapps/remix/app/components/forms/branding-preferences-form.tsxFormMessagecomponents for validation errors. Updated Zod schemas to use Linguimsgfor internationalization of error messages.apps/remix/app/components/forms/signup.tsxoidcProviderLabelprop.apps/remix/app/components/general/analytics/analytics-activity-table-card.tsxapps/remix/app/components/general/analytics/analytics-documents-over-time-card.tsxapps/remix/app/components/general/analytics/analytics-hydrate-fallback.tsxapps/remix/app/components/general/analytics/analytics-no-activity-alert.tsxapps/remix/app/components/general/analytics/analytics-overview-cards.tsxapps/remix/app/components/general/analytics/analytics-page-header.tsxapps/remix/app/components/general/analytics/analytics-query-error.tsxapps/remix/app/components/general/analytics/analytics-range-picker.tsx📊 Architectural Flow
sequenceDiagram participant Parent as Parent App participant Embed as Embed Signing Page participant UI as Embed UI Components Note over Embed,UI: Document Completion/Rejection Flow alt Document Completed Embed->>UI: Render EmbedDocumentCompleted Embed->>Parent: postMessage({ action: 'document-completed', data: {...} }) else Document Rejected Embed->>UI: Render EmbedDocumentRejected Embed->>Parent: postMessage({ action: 'document-rejected', data: {...} }) end Note over Parent: Parent App can now react to signing status changes