-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix(Documenso): Resolve pending status issue for Documenso deployments (fixes #1767 #7145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Conversation
Fix/documenso signing status issue 1767
|
Warning Rate limit exceeded@Aj7Ay has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 12 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughDocker Compose for Documenso replaced implicit placeholders with explicit localhost defaults, added local signing transport and passphrase handling, inlined OpenSSL certificate generation in the entrypoint (with /app/certs fallback), tuned healthchecks/startup timing, and added dev-friendly Postgres defaults and a persistent volume. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Docker as Docker Compose
participant Container as Documenso Container
participant Entrypoint as Entrypoint (inlined)
participant OpenSSL as OpenSSL
participant App as Documenso App
note over Entrypoint,OpenSSL: Inline cert generation + passphrase file
Docker->>Container: create & start
Container->>Entrypoint: execute entrypoint
Entrypoint->>Entrypoint: ensure /app/certs (fallback /tmp/certs)
Entrypoint->>Entrypoint: prepare passphrase file (from env)
Entrypoint->>OpenSSL: generate private.key
OpenSSL-->>Entrypoint: private.key
Entrypoint->>OpenSSL: create CSR & certificate (CN=localhost)
OpenSSL-->>Entrypoint: certificate.crt
Entrypoint->>OpenSSL: export cert.p12 using passphrase file
OpenSSL-->>Entrypoint: cert.p12
Entrypoint->>Entrypoint: chown/chmod certs, update env path if needed
Entrypoint->>Entrypoint: restore cwd
Entrypoint->>App: start app with NEXT_PRIVATE_SIGNING_TRANSPORT=local
App->>Entrypoint: access cert at /app/certs/cert.p12 for signing
note right of App: Postgres service started (with volume) and healthchecks applied
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
templates/compose/documenso.yaml (1)
49-102: Add error handling to prevent container start on certificate generation failure.The entrypoint script lacks error checking. If any OpenSSL command fails (lines 78–94), the script silently continues and executes
./start.shwithout a valid certificate, causing runtime failures instead of fast failure. This aligns with the reported "No session found" errors—the certificate may not have been generated correctly.Add error handling at the start of the script:
entrypoint: - /bin/sh - -c - | + set -e + set -u + trap 'echo "Certificate generation failed"; exit 1' ERR + CERT_DIR="/app/certs"This ensures the container fails immediately if certificate generation encounters an error, making debugging easier and preventing the app from starting without proper signing certificates.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
templates/compose/documenso.yaml(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/README.mdc:0-0
Timestamp: 2025-10-15T09:13:00.453Z
Learning: Applies to docker-compose.dev.yml : Use Docker-first local development via docker-compose.dev.yml
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/development-workflow.mdc:0-0
Timestamp: 2025-08-27T15:01:58.710Z
Learning: Applies to docker-compose.dev.yml : Maintain development Docker services and volumes in docker-compose.dev.yml for local environment parity
🔇 Additional comments (4)
templates/compose/documenso.yaml (4)
31-33: Verify SERVICE_PASSWORD_DOCUMENSO variable availability.The configuration references
${SERVICE_PASSWORD_DOCUMENSO}as the signing passphrase, which differs from typical Coolify variable naming patterns. Confirm this variable is populated by Coolify in the deployment context.Can you verify:
- Is
SERVICE_PASSWORD_DOCUMENSOthe correct variable name provided by Coolify?- Should this reference the main service password or a distinct Documenso password?
56-57: Verify ORIGINAL_DIR logic matches expected container startup behavior.The script saves and restores the working directory before running
./start.sh. Confirm that:
- The initial working directory (saved at line 57) is where
./start.shshould execute.- Changing to
$CERT_DIR(line 76) and back (line 100) doesn't cause unintended side effects.If
./start.shexpects to run from a specific directory (e.g., the app root), verify this is preserved. You may want to add a debug log:# Return to original directory before starting application cd "$ORIGINAL_DIR" + echo "Starting from: $(pwd)" ./start.shAlso applies to: 76-76, 99-100
96-97: UID 1001 is correct for the Documenso container.The Documenso Docker container runs as UID 1001 (the nodejs user), and the project's docs and issue threads reference chown/chmod steps using 1001:1001. Lines 96–97 are properly configured and require no changes.
88-94: Verify OpenSSL version in target Docker image.The
-legacyflag requires OpenSSL 3.0+. Public documentation fordocumenso/documenso:v1.12.10does not specify the included OpenSSL version. Confirm compatibility by:
- Inspecting the Documenso Dockerfile for base image and OpenSSL version, or
- Running the image:
docker run --rm documenso/documenso:v1.12.10 openssl versionIf the image includes OpenSSL < 3.0, remove the
-legacyflag from line 93.
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
templates/compose/documenso.yaml (1)
114-114: Resolve unsafe passphrase parameter substitution.Line 114's
pass:"$$CERT_PASSPHRASE"is vulnerable to shell injection if the passphrase contains special characters (colons, quotes, backslashes, spaces). A previous review flagged this issue; the recommended fix is a file-based approach that safely handles all characters.Apply this diff to use a file-based passphrase approach:
- CERT_PASSPHRASE="$${NEXT_PRIVATE_SIGNING_LOCAL_FILE_PASSPHRASE}" - # Save original working directory ORIGINAL_DIR="$$(pwd)" + + # Write passphrase to a temporary file for safe handling + PASSPHRASE_FILE="/tmp/cert_passphrase" + echo -n "$${NEXT_PRIVATE_SIGNING_LOCAL_FILE_PASSPHRASE}" > "$$PASSPHRASE_FILE" + chmod 600 "$$PASSPHRASE_FILE"Then update the pkcs12 generation:
$$OPENSSL_CMD pkcs12 \ -export \ -out cert.p12 \ -inkey private.key \ -in certificate.crt \ -legacy \ - -passout pass:"$$CERT_PASSPHRASE" + -passin file:"$$PASSPHRASE_FILE" \ + -passout file:"$$PASSPHRASE_FILE" + + # Clean up passphrase file + rm -f "$$PASSPHRASE_FILE"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
templates/compose/documenso.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/README.mdc:0-0
Timestamp: 2025-10-15T09:13:00.453Z
Learning: Applies to docker-compose.dev.yml : Use Docker-first local development via docker-compose.dev.yml
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/development-workflow.mdc:0-0
Timestamp: 2025-08-27T15:01:58.710Z
Learning: Applies to docker-compose.dev.yml : Maintain development Docker services and volumes in docker-compose.dev.yml for local environment parity
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/project-overview.mdc:0-0
Timestamp: 2025-08-27T15:04:22.911Z
Learning: Adopt a Docker-native, container-first deployment approach (Docker Compose, images, volumes, networks)
📚 Learning: 2025-08-27T15:01:58.710Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/development-workflow.mdc:0-0
Timestamp: 2025-08-27T15:01:58.710Z
Learning: Applies to docker-compose.dev.yml : Maintain development Docker services and volumes in docker-compose.dev.yml for local environment parity
Applied to files:
templates/compose/documenso.yaml
📚 Learning: 2025-10-15T09:13:00.453Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/README.mdc:0-0
Timestamp: 2025-10-15T09:13:00.453Z
Learning: Applies to docker-compose.dev.yml : Use Docker-first local development via docker-compose.dev.yml
Applied to files:
templates/compose/documenso.yaml
🔇 Additional comments (1)
templates/compose/documenso.yaml (1)
54-128: Good defensive design for certificate generation fallback.The entrypoint script handles graceful fallback to
/tmp/certsif/app/certsis not writable (lines 74–79), verifies OpenSSL availability before use (lines 67–70), preserves working directory context (lines 60–61, 126), and sets appropriate file permissions for the running user (lines 117–118). This demonstrates good error handling and containerized design patterns.Once the two critical issues above (CN field and passphrase handling) are resolved, this implementation will provide a robust foundation for local certificate-based signing.
nextbranch as the destination for my PR, notmain.Changessection.Issuessection with the issue/discussion link(s) (if applicable).Changes
coolify/templates/compose/documenso.yaml:NEXT_PRIVATE_SIGNING_TRANSPORTis set tolocal.NEXT_PRIVATE_SIGNING_LOCAL_FILE_PATHto/app/certs/cert.p12, which is the standardized location for the generated certificate.NEXT_PRIVATE_SIGNING_LOCAL_FILE_PASSPHRASEto correctly reference${SERVICE_PASSWORD_DOCUMENSO}, ensuring the passphrase is dynamically pulled from Coolify's service configuration.opensslcommands to dynamically generate a self-signed PKCS#12 (.p12) certificate.cat <<INNEREOF > /tmp/cert_info_pathheredoc block to dynamically create the OpenSSL configuration file, allowing for variable substitution.chown 1001:1001andchmod 400commands to correctly set file ownership and permissions, making the certificates accessible to the Documenso application which runs as user1001.ORIGINAL_DIR="$(pwd)"andcd "$ORIGINAL_DIR"to save and restore the working directory, ensuring the final./start.shcommand is executed from the correct context.Issues
Summary by CodeRabbit