feat: Introduce shell metacharacter escaping for exec

[servusdei2018]

In order to mitigate potential security vulnerabilities arising from shell injection attacks, this PR introduces a function to escape shell metacharacters which may be present in command-line arguments.

It's worth noting that two potential vulnerabilities still exist in

nvidia-container-toolkit/tools/container/nvidia-toolkit/run.go

Lines 249 to 252 in 973a663

	cmdline := fmt.Sprintf("%v setup %v %v\n", o.runtime, o.runtimeArgs, toolkitDir)
	//nolint:gosec // TODO: Can we harden this so that there is less risk of command injection
	cmd := exec.Command("sh", "-c", cmdline)

and

nvidia-container-toolkit/tools/container/nvidia-toolkit/run.go

Lines 275 to 278 in 973a663

	cmdline := fmt.Sprintf("%v cleanup %v %v\n", o.runtime, o.runtimeArgs, toolkitDir)
	//nolint:gosec // TODO: Can we harden this so that there is less risk of command injection
	cmd := exec.Command("sh", "-c", cmdline)

where the string o.runtimeArgs is directly interpolated into cmdline, leaving it susceptible to injection attacks. To address this, a more comprehensive solution would involve reimplementing o.runtimeArgs as []string, allowing for proper sanitization using the introduced oci.Escape() function; however, this likely involves a breaking change.

[servusdei2018]

cc @elezar

[elezar]

My one question would be whether the logic that we're implementing here exists in some third-pary package somewhere. I'm sure that runc for example needs to also check the arguments that are passed to its hooks.

[servusdei2018]

Hmm I believe I did look at runc and couldn't find the exact logic they used. I also looked at Docker and they use shellescape internally.

[ArangoGutierrez]

Hey @servusdei2018, let's revise this valuable contribution. Recently, we have done a lot of hardening work, so it's worth revisiting your contribution, starting with a rebase

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[servusdei2018]

Rebased @ArangoGutierrez

[Copilot]

Pull Request Overview

This PR introduces functions to escape shell metacharacters in command-line arguments and updates various exec invocations to use these escaping utilities to mitigate injection risks.

• Adds EscapeArg and Escape functions with accompanying tests.
• Updates syscall.Exec and exec.Command calls across multiple packages to use escaped arguments.
• Includes a new test suite for argument escaping behavior.

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
internal/oci/runtime_syscall_exec.go	Implements EscapeArg and Escape; applies escaping in Exec
internal/oci/runtime_syscall_exec_test.go	Adds tests for the Escape function
internal/ldconfig/safe-exec_other.go	Applies oci.Escape to syscall.Exec in non-Linux SafeExec
internal/ldconfig/safe-exec_linux.go	Applies oci.Escape to both branches of SafeExec on Linux
cmd/nvidia-ctk-installer/.../container.go	Uses oci.Escape before building exec.Command
cmd/nvidia-container-runtime/main_test.go	Updates tests to use oci.EscapeArg and oci.Escape
cmd/nvidia-container-runtime-hook/main.go	Applies oci.Escape before syscall.Exec in hook logic

Comments suppressed due to low confidence (3)

internal/oci/runtime_syscall_exec_test.go:42

• Current tests cover spaces and pipes but miss other metacharacters like backticks, dollar signs, semicolons, quotes, and newlines. Add cases for those to ensure EscapeArg handles all defined metacharacters.

			input:    []string{"echo", "Hello World", "and", "goodbye | cat"},

internal/oci/runtime_syscall_exec.go:65

• [nitpick] Clarify in the docstring that Escape is intended for commands passed through a shell, not for direct use with syscall.Exec or exec.Command without a shell, to avoid confusion about its purpose.

// Escape escapes shell metacharacters in a slice of command-line arguments

internal/oci/runtime_syscall_exec.go:42

• Escaping shell metacharacters is unnecessary when calling syscall.Exec directly, as execve bypasses the shell and does not interpret these characters. This transformation will corrupt arguments like spaces or dollar signs. Consider removing Escape here and relying on direct exec without shell escaping.

	args = Escape(args)

[ArangoGutierrez]

Can you prepend to the commits that start with nit a prefix [no-relnote] , thanks