Skip to content

fix(security): replace NodeSource setup script with direct apt repo config#742

Open
brianwtaylor wants to merge 1 commit intoNVIDIA:mainfrom
brianwtaylor:fix/nodesource-supply-chain
Open

fix(security): replace NodeSource setup script with direct apt repo config#742
brianwtaylor wants to merge 1 commit intoNVIDIA:mainfrom
brianwtaylor:fix/nodesource-supply-chain

Conversation

@brianwtaylor
Copy link
Contributor

@brianwtaylor brianwtaylor commented Mar 23, 2026

Summary

Replace curl | sudo bash of NodeSource's setup_22.x script with GPG key import + apt source configuration in both scripts/brev-setup.sh and scripts/install.sh.

This eliminates execution of a third-party shell script as root entirely. Instead, Node.js is installed via apt's built-in GPG signature verification — the same pattern already used for the NVIDIA Container Toolkit in brev-setup.sh (lines 61–65).

File What changed
scripts/brev-setup.sh NodeSource setup_22.x → GPG key + apt source
scripts/install.sh NodeSource setup_22.x → GPG key + apt source

Test plan

  • Manual: scripts/install.sh on Linux with NODE_MGR=nodesource installs Node.js 22 correctly
  • Manual: scripts/brev-setup.sh on fresh Brev VM installs Node.js correctly
  • Verify /usr/share/keyrings/nodesource.gpg and /etc/apt/sources.list.d/nodesource.list are created
  • Verify node --version reports v22.x after install

Summary by CodeRabbit

  • Chores
    • Updated Node.js 22.x installation configuration in setup and installation scripts to use explicit package repository management instead of automated setup scripts.

…onfig

Replace `curl | sudo bash` of NodeSource's setup_22.x script with GPG key
import + apt source configuration. This eliminates execution of a third-party
shell script as root — the same pattern already used for the NVIDIA Container
Toolkit in brev-setup.sh (lines 61-65).
@coderabbitai
Copy link

coderabbitai bot commented Mar 23, 2026

📝 Walkthrough

Walkthrough

Both installation scripts replace remote Nodesource setup script piping with explicit GPG key management and signed APT repository configuration. The changes add GPG key import, dearmor operations, and repository entry creation before running apt-get update and nodejs installation.

Changes

Cohort / File(s) Summary
Node.js Installation via APT
scripts/brev-setup.sh, scripts/install.sh
Refactored Node.js 22.x installation from piping Nodesource setup script directly into bash to explicit GPG key management with signed APT repository entry. Added GPG key import/dearmor, /usr/share/keyrings/nodesource.gpg configuration, and apt-get update invocation before package installation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Hop, hop! The keys are now in place,
No piping scripts into bash's embrace,
GPG guards our Node with care,
Security updates fill the air!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: replacing NodeSource's remote setup script with direct apt repository configuration for improved security.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch fix/nodesource-supply-chain

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

Copy link

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

🧹 Nitpick comments (1)
scripts/brev-setup.sh (1)

39-43: Good security improvement: explicit GPG key handling replaces curl | bash.

This correctly follows the same pattern as the NVIDIA Container Toolkit setup below (lines 65-69). The signed-by directive ensures apt verifies package signatures against the imported key.

One minor consideration: if the script fails mid-execution and is re-run while node is still not installed, gpg --dearmor -o /usr/share/keyrings/nodesource.gpg will fail because the file already exists. Consider adding --yes to overwrite or removing the file first for idempotency:

  curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key \
-    | sudo gpg --dearmor -o /usr/share/keyrings/nodesource.gpg
+    | sudo gpg --batch --yes --dearmor -o /usr/share/keyrings/nodesource.gpg

This matches better shell script practices for re-runnability, though the existing NVIDIA toolkit block has the same pattern.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/brev-setup.sh` around lines 39 - 43, The gpg dearmor step (curl ... |
sudo gpg --dearmor -o /usr/share/keyrings/nodesource.gpg) is not idempotent and
will fail if /usr/share/keyrings/nodesource.gpg already exists; make the
nodesource key import idempotent by either removing the existing
/usr/share/keyrings/nodesource.gpg before running gpg --dearmor or invoking gpg
with an overwrite flag (e.g., --yes) when writing the key, and ensure the
subsequent creation of /etc/apt/sources.list.d/nodesource.list still uses the
signed-by directive as shown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@scripts/brev-setup.sh`:
- Around line 39-43: The gpg dearmor step (curl ... | sudo gpg --dearmor -o
/usr/share/keyrings/nodesource.gpg) is not idempotent and will fail if
/usr/share/keyrings/nodesource.gpg already exists; make the nodesource key
import idempotent by either removing the existing
/usr/share/keyrings/nodesource.gpg before running gpg --dearmor or invoking gpg
with an overwrite flag (e.g., --yes) when writing the key, and ensure the
subsequent creation of /etc/apt/sources.list.d/nodesource.list still uses the
signed-by directive as shown.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8433bdf5-8f2c-43af-a953-26c806874fe6

📥 Commits

Reviewing files that changed from the base of the PR and between a1e7485 and 3ece6d3.

📒 Files selected for processing (2)
  • scripts/brev-setup.sh
  • scripts/install.sh

@wscurran wscurran added bug Something isn't working CI/CD Use this label to identify issues with NemoClaw CI/CD pipeline or GitHub Actions. security Something isn't secure labels Mar 24, 2026
@wscurran
Copy link
Contributor

Thanks for submitting this PR, it addresses a security issue by replacing the NodeSource setup script with a more secure approach using apt repo config, which improves the overall security of the NemoClaw installation process.

@drobison00
Copy link

Please rebase this PR against the latest main.

@drobison00 drobison00 self-assigned this Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working CI/CD Use this label to identify issues with NemoClaw CI/CD pipeline or GitHub Actions. security Something isn't secure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants