Skip to content

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

Merged
cv merged 1 commit into
NVIDIA: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 into
NVIDIA:mainfrom
dumko2001:fix/h4-env-chmod-brev-deploy

Conversation

@dumko2001

@dumko2001 dumko2001 commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

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 capability or improvement request security Potential vulnerability, unsafe behavior, or access risk 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.

@cv

cv commented Mar 21, 2026

Copy link
Copy Markdown
Collaborator

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

coderabbitai Bot commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

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

@drobison00 drobison00 left a comment

Copy link
Copy Markdown

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

cv commented Mar 24, 2026

Copy link
Copy Markdown
Collaborator

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
jacobtomlinson pushed a commit to jacobtomlinson/NemoClaw that referenced this pull request Apr 30, 2026
@wscurran wscurran added feature PR adds or expands user-visible functionality and removed priority: high enhancement New capability or improvement request labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature PR adds or expands user-visible functionality security Potential vulnerability, unsafe behavior, or access risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants