Skip to content

Reject non-loopback bind addresses for local engine starts - #81

Open
mkalkere wants to merge 3 commits into
NVIDIA:developfrom
mkalkere:fix/engine-bind-validation
Open

mkalkere wants to merge 3 commits into
NVIDIA:developfrom
mkalkere:fix/engine-bind-validation

Conversation

@mkalkere

@mkalkere mkalkere commented Sep 14, 2026 •

Copy link
Copy Markdown

Changelog title

Local engine starts reject non-loopback bind addresses

Changelog body

  • engine:start and engine:install now reject non-loopback bind overrides with -32602, closing an accidental LAN exposure of the unauthenticated engine API. Remote starts already hard-bind loopback; local starts now match.

Bumps

  • services: patch
  • nvpair-cluster-manager: none
  • nvpair-engine-manager: patch
  • 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

Description

engine:start / engine:install accepted any valid IP as the bind override, which put an unauthenticated engine API on 0.0.0.0. Remote starts already hard-bind 127.0.0.1; local starts now reject non-loopback binds with -32602 "bind must be a loopback address".

Scope

Included: services/nvpair-engine-manager bind validation, bind_validation_test.go, release-intent bump declaration, docs/engine-bind-validation.mdx. Nothing else touched.

Validation

  • go test -race ./... in services/nvpair-engine-manager: pass (full suite ~49s).
  • New tests: 0.0.0.0, 192.168.1.5, :: rejected; invalid IPs still rejected as invalid; loopback passes.
  • Toolchain: Go 1.26.8 (repo requires Go 1.25+), Linux sandbox. go vet clean, gofmt clean, node scripts/spdx-headers.mjs reports 0 missing headers on every branch.

Risk

  • Anyone deliberately binding a local engine to a LAN address now gets an error. That was never a supported configuration (the remote path hard-binds loopback), so this closes an accidental exposure, not a feature.

Checklist

  • I have read the Contributing Guidelines.
  • Every commit is signed off (git commit -s), certifying the Developer Certificate of Origin.
  • New or existing tests cover the change.
  • Relevant documentation is updated.
  • I checked the diff, changed filenames, and commit messages for credentials, private data, internal URLs, internal issue identifiers, and generated artifacts.
  • I recorded the validation commands and results above.
  • I declared the component bump in the pair-release-intent:v1 block above (services/versions.json is automation-managed; CI rejects hand edits), and described user-visible changes so they reach the release notes.

@Noah-Tervalon-Nvidia
Noah-Tervalon-Nvidia changed the base branch from main to develop September 21, 2026 21:56
engine:start accepted any valid IP as a bind override, silently exposing
the unauthenticated engine API to the LAN. Non-loopback binds now fail
with -32602, matching the remote-start path that hard-binds 127.0.0.1.

Signed-off-by: Mallikh Kaula <mallikh@users.noreply.github.com>
Validation flow for the loopback bind requirement, plus a reading-order
entry in the README.

Signed-off-by: Mallikh Kaula <mallikh@users.noreply.github.com>
NewManager now wires the executor into the settings relay at
construction, so the test fixture builds it with a real Executor and
nils it back out right after to keep the panic-on-reach signal.

Signed-off-by: Mallikh Kaula <mallikh@users.noreply.github.com>
@mkalkere
mkalkere force-pushed the fix/engine-bind-validation branch from 5938625 to 77bd271 Compare September 26, 2026 19:33
@mkalkere

Copy link
Copy Markdown
Author

Same rebase as #80 — onto current develop, versions.json bump dropped in favor of the release-intent declaration. One adaptation was needed: NewManager now wires the executor into the settings relay at construction, which broke the test fixture's nil-executor trick, so the fixture builds a real Executor and nils it back out right after (separate commit, same panic-on-reach behavior). Full module suite passes with -race.

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