Skip to content

fix(deploy): ensure remote .env file has secure permissions (600)#174

Merged
cv merged 1 commit intoNVIDIA:mainfrom
dumko2001:fix/h4-env-chmod-brev-deploy
Mar 24, 2026
Merged

fix(deploy): ensure remote .env file has secure permissions (600)#174
cv merged 1 commit intoNVIDIA:mainfrom
dumko2001:fix/h4-env-chmod-brev-deploy

Conversation

@dumko2001
Copy link
Copy Markdown
Contributor

@dumko2001 dumko2001 commented Mar 17, 2026

Rationale

The .env file on the remote deployment target could be uploaded with insecure default permissions, exposing secrets.

Changes

Added a post-deployment step to explicitly enforce 600 permissions on the remote .env file.

Verification Results

  • Automated Tests: Passed all 52 core tests via npm test.
  • Manual Audit: Verified remote .env permissions after deployment.
  • Security Review: Verified correct permission enforcement.

Leading Standards

This PR follows the project's 'First Principles' approach, prioritizing deterministic behavior and zero-trust security defaults.

Summary by CodeRabbit

  • Bug Fixes
    • Environment configuration files deployed to remote servers are now automatically secured after transfer by applying stricter file permissions, reducing risk of accidental exposure.

@wscurran wscurran added enhancement New feature or request security Something isn't secure labels Mar 19, 2026
@wscurran
Copy link
Copy Markdown
Contributor

I've taken a look at the changes and see that this PR adds a post-deployment step to enforce 600 permissions on the remote .env file, which should prevent insecure default permissions from exposing secrets.

@wscurran wscurran added the priority: high Important issue that should be resolved in the next release label Mar 19, 2026
@cv
Copy link
Copy Markdown
Contributor

cv commented Mar 21, 2026

Hey @dumko2001 — securing .env permissions to 600 is a solid improvement. Would you be able to rebase onto the latest main? The repo has been moving quickly and we want to evaluate this against the current state of things. Thanks!

@dumko2001 dumko2001 force-pushed the fix/h4-env-chmod-brev-deploy branch from 59a65ca to d08cf0e Compare March 21, 2026 19:38
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 21, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 73ea356e-99d5-4d5c-b232-d611525525a4

📥 Commits

Reviewing files that changed from the base of the PR and between 9aa83b4 and 748665e.

📒 Files selected for processing (1)
  • bin/nemoclaw.js
✅ Files skipped from review due to trivial changes (1)
  • bin/nemoclaw.js

📝 Walkthrough

Walkthrough

The deployment script now, after copying the local .env to the remote VM via scp, runs an additional remote ssh command to execute chmod 600 /home/ubuntu/nemoclaw/.env on the target host.

Changes

Cohort / File(s) Summary
Deployment Permission Hardening
bin/nemoclaw.js
After scp of the generated local .env to /home/ubuntu/nemoclaw/.env, invoke a remote ssh command to run chmod 600 /home/ubuntu/nemoclaw/.env on the target host.

Sequence Diagram(s)

sequenceDiagram
    participant Local as Local Deploy Script
    participant SCP as scp
    participant SSH as ssh
    participant Remote as Remote VM

    rect rgba(200,200,255,0.5)
    Local->>SCP: transfer .env to /home/ubuntu/nemoclaw/.env
    SCP-->>Remote: write file
    end

    rect rgba(200,255,200,0.5)
    Local->>SSH: run "chmod 600 /home/ubuntu/nemoclaw/.env"
    SSH-->>Remote: change file permissions
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 I hopped a line into the night,
Sent secrets safe, then pulled them tight,
Six hundred locks to dim the light,
Quiet paws keep configs right. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding secure permissions (600) to the remote .env file during deployment, which directly addresses the security vulnerability outlined in the PR objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@dumko2001 dumko2001 force-pushed the fix/h4-env-chmod-brev-deploy branch from d08cf0e to 9aa83b4 Compare March 21, 2026 19:45
@dumko2001
Copy link
Copy Markdown
Contributor Author

@cv hey successfully rebased.check it out when you have time

@wscurran wscurran requested a review from drobison00 March 23, 2026 16:42
@drobison00 drobison00 self-assigned this Mar 23, 2026
Copy link
Copy Markdown

@drobison00 drobison00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another good permissions catch

@dumko2001
Copy link
Copy Markdown
Contributor Author

@drobison00 thanks

@cv
Copy link
Copy Markdown
Contributor

cv commented Mar 24, 2026

Not a duplicate of #186 (merged). #186 adds `chmod 600 .env` inside the sandbox during startup. This PR adds it on the remote host after SCP during `deploy()`. Different locations protecting different files.

The deploy function has been restructured in #691 (SSH TOFU) — you'll need to rebase. The fix itself is a valid one-liner that #691 doesn't include.

@dumko2001 dumko2001 force-pushed the fix/h4-env-chmod-brev-deploy branch from 9aa83b4 to 748665e Compare March 24, 2026 07:26
@dumko2001
Copy link
Copy Markdown
Contributor Author

@cv have rebased, pls take a look

@dumko2001 dumko2001 requested a review from drobison00 March 24, 2026 07:28
@cv cv merged commit 9788244 into NVIDIA:main Mar 24, 2026
4 of 5 checks passed
jessesanford pushed a commit to jessesanford/NemoClaw that referenced this pull request Mar 24, 2026
temrjan pushed a commit to temrjan/NemoClaw that referenced this pull request Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request priority: high Important issue that should be resolved in the next release security Something isn't secure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants