Skip to content

fix: match the root at a path component boundary when transforming paths - #1955

Open
vyncint wants to merge 1 commit into
NVIDIA:mainfrom
vyncint:fix-root-transform-path-boundary
Open

fix: match the root at a path component boundary when transforming paths#1955
vyncint wants to merge 1 commit into
NVIDIA:mainfrom
vyncint:fix-root-transform-path-boundary

Conversation

@vyncint

@vyncint vyncint commented Jul 29, 2026

Copy link
Copy Markdown

Problem

pkg/nvcdi/transform/root decides whether a path is contained in the (from) root using a plain string prefix comparison:

if !strings.HasPrefix(path, t.root) {
	return path
}

return filepath.Join(t.targetRoot, strings.TrimPrefix(path, t.root))

A path that shares a textual prefix with the root but is not actually inside it therefore matches, and is rewritten into a corrupted path.

With root=/run/nvidia/driver and targetRoot=/host:

path current expected
/run/nvidia/driver/lib/libcuda.so /host/lib/libcuda.so /host/lib/libcuda.so
/run/nvidia/driver-backup/lib/libcuda.so /host/-backup/lib/libcuda.so unchanged
/run/nvidia/driverfoo /host/foo unchanged

The last two paths lie outside the root and must be left untouched.

Reachability

transformPath is shared by hostRootTransformer and containerRootTransformer, so both host and container paths are affected. The roots are user-supplied:

  • nvidia-ctk cdi transform root --from <root> --to <target>
  • the driver root transform applied during CDI spec generation (NewDriverTransformer, driven by --driver-root / --dev-root)

Fix

Require the prefix match to end at a path component boundary. Behaviour that was already correct is unchanged:

  • a path equal to the root still maps to the target root
  • a root with a trailing separator, including a root of /, still matches
  • a path with no common prefix is still returned untouched

Testing

Added TestTransformPath covering the two boundary cases plus five cases for the existing behaviour. I verified that the two new cases fail without the fix and pass with it, and that the other five pass either way.

Locally, on a machine with no NVIDIA GPU:

  • make test — pass (44 packages)
  • golangci-lint run ./... — 0 issues
  • golangci-lint fmt --diff ./... — clean
  • go build ./... — pass

Coverage for pkg/nvcdi/transform/root: 89.9% -> 90.2%.

Out of scope

The same prefix-comparison pattern also appears in internal/lookup/root.RelativeToRoot, internal/discover/mounts.go, and pkg/nvcdi/driver-nvml.go. I have left those untouched to keep this change small and reviewable, but I am happy to follow up if you would like them addressed as well.

The root transformer used a plain string prefix comparison to determine
whether a path is contained in the (from) root. A path that shares a
textual prefix with the root but lies outside it was therefore treated as
being under the root and rewritten, corrupting the path.

For a root of /run/nvidia/driver and a target root of /host:

	/run/nvidia/driver-backup/lib.so -> /host/-backup/lib.so
	/run/nvidia/driverfoo            -> /host/foo

Both paths are outside the root and must be left unchanged.

The prefix match is now required to end at a path component boundary. A
path equal to the root, and a root with a trailing separator (including a
root of /), are handled as before.

This applies to both the host and container root transformers since they
share transformPath, and is reachable from
'nvidia-ctk cdi transform root --from/--to' as well as from the driver
root transform applied during CDI spec generation.

Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 29, 2026 07:27
@copy-pr-bot

copy-pr-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown

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.

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants