Skip to content

Conversation

@Aj7Ay
Copy link

@Aj7Ay Aj7Ay commented Nov 7, 2025

  • I have selected the next branch as the destination for my PR, not main.
  • I have listed all changes in the Changes section.
  • I have filled out the Issues section with the issue/discussion link(s) (if applicable).
  • I have tested my changes.
  • I have considered backwards compatibility.

Changes

  • Updated coolify/templates/compose/documenso.yaml:
    • Environment Variables:
      • Ensured NEXT_PRIVATE_SIGNING_TRANSPORT is set to local.
      • Corrected NEXT_PRIVATE_SIGNING_LOCAL_FILE_PATH to /app/certs/cert.p12, which is the standardized location for the generated certificate.
      • Corrected NEXT_PRIVATE_SIGNING_LOCAL_FILE_PASSPHRASE to correctly reference ${SERVICE_PASSWORD_DOCUMENSO}, ensuring the passphrase is dynamically pulled from Coolify's service configuration.
    • Entrypoint Script Logic:
      • Implemented a series of openssl commands to dynamically generate a self-signed PKCS#12 (.p12) certificate.
      • Utilized a cat <<INNEREOF > /tmp/cert_info_path heredoc block to dynamically create the OpenSSL configuration file, allowing for variable substitution.
      • Added chown 1001:1001 and chmod 400 commands to correctly set file ownership and permissions, making the certificates accessible to the Documenso application which runs as user 1001.
      • Implemented ORIGINAL_DIR="$(pwd)" and cd "$ORIGINAL_DIR" to save and restore the working directory, ensuring the final ./start.sh command is executed from the correct context.
  • Local Verification:
    • The entire entrypoint script was thoroughly tested in a simulated Docker container environment, confirming that all steps execute successfully and as expected.

Issues

Summary by CodeRabbit

  • Chores
    • Use explicit local defaults for service URLs and ports to simplify local runs.
    • Consolidated certificate creation to inline OpenSSL generation with standardized filenames, passphrase support (keeps secrets out of process list), explicit cert directory with writable fallback and permission handling.
    • Added a service password entry and updated certificate metadata defaults for local development.
    • Added a persistent database volume and adjusted dev-friendly DB defaults.
    • Tuned startup and healthcheck timings for more reliable local startup.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 7, 2025

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 40eb399 and e3c3962.

📒 Files selected for processing (1)
  • templates/compose/documenso.yaml (5 hunks)

Walkthrough

Docker 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

Cohort / File(s) Summary
Compose: Documenso service
templates/compose/documenso.yaml
Replaced placeholder envs with explicit localhost defaults (e.g., SERVICE_URL_DOCUMENSO, NEXTAUTH_URL, NEXT_PUBLIC_WEBAPP_URL); added port mapping; added SERVICE_PASSWORD_DOCUMENSO; set dev-friendly Postgres env defaults; declared volumes: and documenso_postgresql_data volume.
Signing transport & envs
templates/compose/documenso.yaml
Added NEXT_PRIVATE_SIGNING_TRANSPORT=local; introduced NEXT_PRIVATE_SIGNING_LOCAL_FILE_PATH=/app/certs/cert.p12 and NEXT_PRIVATE_SIGNING_LOCAL_FILE_PASSPHRASE (sourced from SERVICE_PASSWORD_DOCUMENSO fallback); updated CERT_INFO defaults (country, state, locality, organization, email) and CN=localhost.
Entrypoint / Inline cert generation
templates/compose/documenso.yaml
Removed external cert-generation script refs; inlined OpenSSL-based certificate creation in entrypoint: ensures /app/certs (falls back to /tmp/certs), uses passphrase file to call OpenSSL, generates private.key, certificate.crt, and cert.p12 (PKCS#12), sets secure permissions/ownership, updates NEXT_PRIVATE_SIGNING_LOCAL_FILE_PATH if dir changed, and restores original cwd before app start.
Healthcheck & startup tuning
templates/compose/documenso.yaml
Healthchecks now target localhost:3000, increased interval/start_period, reduced timeout/retries; startup flow adjusted so app waits for cert setup; Postgres healthcheck defaults expanded and start_period added.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Verify OpenSSL commands, passphrase-file usage, and that passphrase is not exposed in process listings.
  • Confirm file/directory permission and ownership choices (400, 1001:1001) are appropriate for runtime user.
  • Check renamed env vars (NEXT_PRIVATE_SIGNING_LOCAL_FILE_PASSPHRASE, SERVICE_PASSWORD_DOCUMENSO) and any external references.
  • Validate healthcheck targets/timing and Postgres volume declaration.

Poem

🐇
In a cozy /app/certs bed I hop,
Keys and certs sprout—no scripts to stop,
Passphrases hidden, permissions tight,
Local signing hums through morning light,
I nibble ports and watch the service hop.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly describes the fix addressing a pending status issue for Documenso deployments and references the specific issue #1767.
Description check ✅ Passed The PR description follows the template structure, includes detailed changes and testing verification, and directly addresses the linked issue.
Linked Issues check ✅ Passed The PR implements certificate generation and environment variable configuration that directly addresses the root cause ('No session found' errors) preventing document status transition [#1767].
Out of Scope Changes check ✅ Passed All changes are focused on fixing the Documenso compose configuration and certificate handling to resolve the session/status issue, with no unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.sh without 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

📥 Commits

Reviewing files that changed from the base of the PR and between 7bbfa09 and b0a17b9.

📒 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:

  1. Is SERVICE_PASSWORD_DOCUMENSO the correct variable name provided by Coolify?
  2. 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:

  1. The initial working directory (saved at line 57) is where ./start.sh should execute.
  2. Changing to $CERT_DIR (line 76) and back (line 100) doesn't cause unintended side effects.

If ./start.sh expects 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.sh

Also 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 -legacy flag requires OpenSSL 3.0+. Public documentation for documenso/documenso:v1.12.10 does not specify the included OpenSSL version. Confirm compatibility by:

  1. Inspecting the Documenso Dockerfile for base image and OpenSSL version, or
  2. Running the image: docker run --rm documenso/documenso:v1.12.10 openssl version

If the image includes OpenSSL < 3.0, remove the -legacy flag from line 93.

@gitguardian
Copy link

gitguardian bot commented Nov 7, 2025

️✅ 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.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between b0a17b9 and 50accfe.

📒 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/certs if /app/certs is 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.

@andrasbacsai andrasbacsai changed the base branch from v4.x to next November 7, 2025 13:12
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.

Status showing Pending , after both parties signed

2 participants