Repository navigation
Harden trust store paths against unsafe node identifiers - #159
Merged
Merged
Conversation
Signed-off-by: Kaylee Lubick <klubick@nvidia.com>
Signed-off-by: Kaylee Lubick <klubick@nvidia.com>
sherief-nv
reviewed
Oct 13, 2026
sherief-nv
reviewed
Oct 13, 2026
Signed-off-by: Kaylee Lubick <klubick@nvidia.com>
Noah-Tervalon-Nvidia
approved these changes
Oct 13, 2026
Noah-Tervalon-Nvidia
pushed a commit
that referenced
this pull request
Oct 29, 2026
<!-- pair-release-intent:v1 --> ### Changelog title Harden trusted-peer file paths ### Changelog body - Reject unsafe peer identifiers before loading, writing, or removing trust-store files, protecting cluster data from path traversal. ### Bumps - services: patch - nvpair-cluster-manager: patch - nvpair-engine-manager: none - nvpair-errors: none - nvpair-job-scheduler: none - nvpair-manual-nodes: none - nvpair-node-info: none - nvpair-node-scanner: none - nvpair-node-settings: none - nvpair-proxy: none - nvpair-tui: none - nvpair-ui-broker: none - nvpair-workload-manager: none <!-- /pair-release-intent:v1 --> ## Summary While reviewing other code, it came up that we use the contents of certificates in a filepath.Join. This could be a vector of a file traversal issue - an endorsed roster entry could supply a traversal identifier and cause a trust-store write outside the trusted directory. Validate identifiers before deriving pin paths. Reject empty identifiers, dot components, either platform's directory separators, Windows drive/stream syntax, NUL bytes, and native reserved filenames. Reject unsafe inputs without cleaning or renaming them, changing trust-store state, or emitting a trust-store change notification. ## Scope - Apply validation to pin loading, certificate validation, persistence, and removal. - Consolidate trust-store tests and add rejection, reload, notification, and signed-roster regression coverage. - Update the cluster-manager specification. No JSON-RPC or on-disk schema changes. - Declare patch bumps for cluster-manager and the services suite because this fixes trust-store path handling. Other components are unchanged. ## Validation Reviewed commit `83588ff` on Windows amd64 with Go 1.27.0: - `go test ./...` in `services/nvpair-cluster-manager` - passed. - `go test -run '^Test(TrustStore|MergeRosterRejectsTraversalPrincipal)' -count=1 -v` in the same module - passed, including Windows reserved-name cases. - Negative control: bypassed the validation predicate using a temporary Go overlay. The signed-roster regression failed by admitting the traversal principal and overwriting the temporary identity file, confirming it detects the original defect. - `git diff --check upstream/develop...HEAD` - passed. - `node scripts/spdx-headers.mjs` - passed, no missing headers. ## Risk Valid generated UUIDs and safe opaque identifiers retain existing pin, reload, and idempotent removal behavior. Unsafe existing pin filenames are skipped on load and left on disk. Certificate principal matching remains required. --------- Signed-off-by: Kaylee Lubick <klubick@nvidia.com> Signed-off-by: Terve <ntervalon@nvidia.com>
7 tasks done
JaredP94
added a commit
to JaredP94/Personal-AI-Router
that referenced
this pull request
Nov 2, 2026
The trusted-node store joined a peer's UUID straight into a file path. The UUID comes from the network — a pairing exchange or a roster entry another trusted member endorsed — so a crafted identifier such as "../identity" could make a roster merge write a pin outside trusted/, overwriting this node's identity.json. Validate identifiers before deriving pin paths: reject empty identifiers, dot components, either platform's separators, Windows drive/stream syntax, NUL bytes and native reserved names. Load skips such files; Pin and Remove refuse them without touching disk or announcing a change. Ported from NVIDIA/Personal-AI-Router@c299135 (NVIDIA#159). The tests are adapted to this fork's test layout. Co-authored-by: Kaylee Lubick <kjlubick@users.noreply.github.com> Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
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.
Changelog title
Harden trusted-peer file paths
Changelog body
Bumps
Summary
While reviewing other code, it came up that we use the contents of certificates in a filepath.Join. This could be a vector of a file traversal issue - an endorsed roster entry could supply a traversal identifier and cause a trust-store write outside the trusted directory.
Validate identifiers before deriving pin paths. Reject empty identifiers, dot components, either platform's directory separators, Windows drive/stream syntax, NUL bytes, and native reserved filenames. Reject unsafe inputs without cleaning or renaming them, changing trust-store state, or emitting a trust-store change notification.
Scope
Validation
Reviewed commit
83588ffon Windows amd64 with Go 1.27.0:go test ./...inservices/nvpair-cluster-manager- passed.go test -run '^Test(TrustStore|MergeRosterRejectsTraversalPrincipal)' -count=1 -vin the same module - passed, including Windows reserved-name cases.git diff --check upstream/develop...HEAD- passed.node scripts/spdx-headers.mjs- passed, no missing headers.Risk
Valid generated UUIDs and safe opaque identifiers retain existing pin, reload, and idempotent removal behavior. Unsafe existing pin filenames are skipped on load and left on disk. Certificate principal matching remains required.