docs: Update for security fixes in PraisonAI PR #1744#455
Conversation
- Update GCP deployment to use secure --env-vars-file pattern - Add gateway auth token precedence documentation - Document sandbox timeout behavior and resource cleanup - Add security hardening documentation for recent fixes - Update troubleshooting guide for auth issues 🤖 Generated with Claude Code Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 42 minutes and 1 second. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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 updates various documentation files to detail security hardening features introduced in PR #1744, including secure cloud deployment secret handling, gateway auth token unification, and sandbox resource cleanup. Feedback highlights several critical discrepancies where the documented behaviors—such as exporting the gateway config token to the environment, launching Docker containers with deterministic names, and cleaning up SSH temp files—do not match the actual codebase implementations. Additionally, a potential race condition was flagged in the manual GCP deployment instructions where sensitive environment files are briefly created with default permissions before being secured.
| cat > /tmp/praisonai-env.yaml <<EOF | ||
| OPENAI_MODEL_NAME: "${OPENAI_MODEL_NAME}" | ||
| OPENAI_API_KEY: "${OPENAI_API_KEY}" | ||
| OPENAI_API_BASE: "${OPENAI_API_BASE}" | ||
| EOF | ||
| chmod 600 /tmp/praisonai-env.yaml |
There was a problem hiding this comment.
Creating the temporary environment file with cat > /tmp/praisonai-env.yaml creates the file with default permissions (typically 0644 or 0666 depending on the system's umask) before chmod 600 is executed. This introduces a brief race condition where other local users could read the sensitive API keys.
Using a subshell with umask 077 ensures that the file is created with secure permissions (0600) from the very beginning, eliminating the race condition.
(umask 077 && cat > /tmp/praisonai-env.yaml <<EOF
OPENAI_MODEL_NAME: "${OPENAI_MODEL_NAME}"
OPENAI_API_KEY: "${OPENAI_API_KEY}"
OPENAI_API_BASE: "${OPENAI_API_BASE}"
EOF
)
| These commands are for manual deployment only. Use `praisonai deploy` for automated deployment. | ||
|
|
||
| <Warning> | ||
| Never pass secrets via `--set-env-vars` — they appear in `ps`, shell history, and CI logs. Use `--env-vars-file` with `chmod 0600`. The `praisonai deploy run` command does this automatically (since PR #1744). |
There was a problem hiding this comment.
The documentation states that praisonai deploy run automatically handles secrets securely using --env-vars-file since PR #1744. However, the actual codebase in praisonai/deploy.py and praisonai/deploy/providers/gcp.py still uses the insecure --set-env-vars pattern.
Please ensure that the corresponding codebase changes are merged or updated to match this documentation, otherwise it will mislead users into believing their deployments are automatically secured.
| | Backend | Local cleanup on timeout | Remote cleanup on timeout | | ||
| |---|---|---| | ||
| | `subprocess` | Process killed | N/A | | ||
| | `docker` | Client process killed **and** `docker kill <container>` issued on the named container (`praisonai-<execution_id>`) | Container stopped — resource limits enforced | | ||
| | `ssh` | Local SSH client killed | Remote process terminated via `timeout N sh -c ...` wrapper; remote temp file cleaned up via `finally` (even on error) | | ||
| | `e2b`, `modal`, `native`, `sandlock`, `daytona` | Backend-managed | Backend-managed | |
There was a problem hiding this comment.
There are significant discrepancies between this documentation and the actual sandbox implementations in the codebase:
- Docker Sandbox (
praisonai/sandbox/docker.py): The code does not launch containers with--name praisonai-<execution_id>(no--nameparameter is passed in_build_docker_command), nor does it issuedocker killon timeout in theasyncio.TimeoutErrorblock. - SSH Sandbox (
praisonai/sandbox/ssh.py): The remote temp file cleanup (rm -f) is not wrapped in afinallyblock inexecute(), meaning that if a timeout or exception occurs, the cleanup command is skipped and the temp file is leaked.
Please update the codebase to implement these security/cleanup features, or correct the documentation to reflect the actual behavior.
|
|
||
| ## Token Source Precedence | ||
|
|
||
| When `GatewayConfig(auth_token=...)` is set, it is exported to `GATEWAY_AUTH_TOKEN` at gateway startup. **Config wins over env.** All auth paths — HTTP login, magic-link verification, WebSocket handshake — read the same secret. |
There was a problem hiding this comment.
The documentation states that GatewayConfig(auth_token=...) is exported to the GATEWAY_AUTH_TOKEN environment variable at gateway startup. However, in praisonai/gateway/server.py, there is no code that writes to os.environ["GATEWAY_AUTH_TOKEN"].
While the config value does take precedence internally over the environment variable, it is not actually exported to the environment. If other subprocesses or components rely on this environment variable being set, they will not receive it. Please update the server implementation to export the token, or update the documentation to clarify.
Summary
Updates documentation for the three security fixes shipped in PraisonAI PR #1744:
Changes
Updated Files
Documentation Standards
Test Plan
Fixes #444
🤖 Generated with Claude Code