Various security fixes - #1
Open
NorseGaud wants to merge 2 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security Audit & Fixes
Date: 2026-02-13
Scope: Full source review of
dgx-spark-cli(15 Go source files, 2 shell scripts, 1 CI workflow, dependency manifest)Malware assessment: Clean — no embedded backdoors, data exfiltration, obfuscated code, or suspicious network calls detected.
Findings Summary
HIGH Severity
1. Insecure SSH Host Key Verification Fallback
File:
internal/ssh/client.goProblem: When
~/.ssh/known_hostsdid not exist or could not be read, the SSH client silently fell back tossh.InsecureIgnoreHostKey()with only a stderr warning. This accepted any host key without user consent, enabling man-in-the-middle attacks on every first connection.Fix: Replaced with an interactive trust-on-first-use (TOFU) flow that mirrors standard OpenSSH behavior. When
known_hostsis missing, the user is prompted to trust the host key. Declining aborts the connection. Accepting scans the key viassh-keyscanand createsknown_hosts. If the file exists but cannot be parsed, the connection is refused with an error.2. Command Injection via Unsanitized User Input
Files:
cmd/dgx/main.go,internal/playbook/ollama.go,internal/playbook/vllm.go,internal/playbook/nvfp4.goProblem: User-supplied values (model names, prompts, file paths) were interpolated directly into shell command strings executed over SSH without any quoting or sanitization. An attacker-controlled value containing shell metacharacters (e.g.,
'; rm -rf / #) could break out of the intended command and execute arbitrary code on the remote DGX.ollama.go—ollamaRunfmt.Sprintf("ollama run %s '%s'", model, prompt)nvfp4.go—nvfp4Quantize--model_name %sinsidebash -c "..."vllm.go—vllmServevllm serve %sinsidedocker runmain.go—ensureRemoteDirectorymkdir -p %swith unquoted pathFix: Introduced a single exported
ssh.ShellQuote()function that wraps values in single quotes with proper embedded-quote escaping. Applied it at every injection point:ollama.go— model and prompt are now individually shell-quoted:vllm.go— model name is shell-quoted in the docker command:nvfp4.go— restructured to pass the model name as a Docker environment variable (-e MODEL_NAME=<quoted>) and switchedbash -cfrom double quotes to single quotes, referencing"$MODEL_NAME"inside the container to completely eliminate the injection surface:main.go— path is now shell-quoted:MEDIUM Severity
3. Unconfirmed Remote Script Execution (curl | sh)
Files:
internal/playbook/ollama.go,internal/playbook/dmr.goProblem: Two playbook commands downloaded and piped remote scripts directly into a shell without any warning or user confirmation:
dgx run ollama installcurl -fsSL https://ollama.com/install.sh | shdgx run dmr setupcurl -fsSL https://get.docker.com | sudo shThe DMR variant ran with
sudo, meaning a compromised upstream script would gain root access on the DGX.Fix: Both commands now display a clear warning about what will be downloaded and require explicit
[Y/n]confirmation before proceeding. Declining cancels the operation cleanly.4. Unsanitized Model Name in Docker Commands
File:
internal/playbook/vllm.goProblem: The model argument to
vllm servewas passed directly into adocker runcommand string without sanitization.Fix: Covered by the
ssh.ShellQuote()fix described in HIGH #2 above. The model name is now properly quoted before interpolation.LOW Severity
5. Duplicate
shellQuoteFunctionFiles:
cmd/dgx/main.go,internal/playbook/dmr.goProblem: Two identical private
shellQuotefunctions existed in separate packages. If one were updated (e.g., to fix an edge case) and the other forgotten, inconsistent quoting could reintroduce injection vulnerabilities.Fix: Both copies removed. A single exported
ssh.ShellQuote()function now lives ininternal/ssh/client.goand is used by all callers across the codebase.6. Hardcoded
--deleteFlag in rsyncFile:
cmd/dgx/main.goProblem: The internal
syncDirectoryToRemotefunction always passed--deleteto rsync, which removes files on the remote that don't exist locally. If pointed at the wrong remote path, this could cause unintended data loss with no way for callers to opt out.Fix: Changed the function signature to accept a
deleteExtraneous boolparameter. The flag is only added when explicitly requested. The existing call site (dgx codex import-config) passestrueto preserve its current behavior.Files Modified
internal/ssh/client.goShellQuote()cmd/dgx/main.goshellQuote; usedssh.ShellQuote; quoted path inensureRemoteDirectory; parameterized--deleteinternal/playbook/ollama.gointernal/playbook/vllm.gointernal/playbook/nvfp4.gointernal/playbook/dmr.goshellQuotewithssh.ShellQuote; removed duplicateREADME.md