Skip to content

[2.8] Restrict remote log configuration and local listeners - #5202

Merged
chesterxgchen merged 7 commits into
NVIDIA:2.8from
pcnudde:feat/2.8-remote-log-loopback-fixes
Aug 25, 2026
Merged

[2.8] Restrict remote log configuration and local listeners#5202
chesterxgchen merged 7 commits into
NVIDIA:2.8from
pcnudde:feat/2.8-remote-log-loopback-fixes

Conversation

@pcnudde

@pcnudde pcnudde commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Testing

  • targeted unit tests: 42 passed
  • project style checks: ./runtest.sh -s

## Summary

- Reuse existing site log validation for remote job-log updates.
- Apply the same controls to server and client job processes.
- Align the API documentation with supported inputs and add regression
coverage.

## Validation

- 21 focused logging and command-routing tests
- `./runtest.sh -s`
- `git diff --check`
Use explicit IPv4 loopback binding for local listeners configured with
`localhost`. Non-localhost configurations retain their existing
defaults.

- Focused listener tests
- Local SimEnv and POC end-to-end tests
- Style checks
@greptile-apps

greptile-apps Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This security backport restricts remote job-log configuration to supported levels and built-in modes while binding local simulation and POC listeners to IPv4 loopback without changing production wildcard defaults.

  • Validates job-log configuration on server and client paths.
  • Propagates client rejection, timeout, missing-response, and partial-response failures through error data and metadata.
  • Separates advertised listener hosts from bind hosts and applies loopback binding to local environments.
  • Adds focused coverage for logging validation, reply handling, and listener selection.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
nvflare/private/fed/server/job_cmds.py Detects rejected, non-OK, absent, and partial client responses and emits both visible error data and error metadata.
nvflare/private/fed/client/admin_commands.py Validates remote job-log configuration before applying it and returns validation failures to the server.
nvflare/private/fed/server/server_commands.py Restricts server-side job-log configuration to supported safe values.
nvflare/fuel/f3/cellnet/connector_manager.py Adds an optional internal listener host while preserving explicitly configured communication resources.
nvflare/fuel/f3/drivers/net_utils.py Separates the advertised connection host from the listener bind host while retaining wildcard binding by default.
nvflare/private/fed/server/fed_server.py Uses IPv4 loopback for localhost deployments while preserving wildcard listener behavior for production targets.

Sequence Diagram

sequenceDiagram
  participant O as Operator/API
  participant S as Server command
  participant C as Client job
  O->>S: configure_job_log
  S->>S: Validate server configuration
  S->>C: Forward validated configuration
  C->>C: Validate and apply configuration
  C-->>S: Success or rejection
  alt All requested clients succeed
    S-->>O: Reply table with successful metadata
  else Rejection, timeout, or missing reply
    S-->>O: Reply table plus error item and error metadata
  end
Loading

Reviews (6): Last reviewed commit: "Report client job log failures to intera..." | Re-trigger Greptile

Comment thread nvflare/private/fed/client/admin_commands.py
pcnudde and others added 3 commits August 25, 2026 14:21
Match main's guard: only apply internal_listener_host when comm_config
does not configure an internal host; use setdefault for listen_host.
Strengthen listener tests with distinct host/listen_host values.
Constrain the 2.8 app_opt dependency to XGBoost versions that still
provide DataSplitMode.COL. XGBoost 3.4 removes column-wise data splitting
and causes import-time failures in the XGBoost integration and recipes
that transitively import it.

Validated with the focused XGBoost and recipe tests on Python 3.14.
Propagate client-side configure_job_log failures through command metadata so API and CLI callers can detect explicit errors, missing replies, and partial fan-out responses. Add focused success and failure regression coverage.
@pcnudde

pcnudde commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator Author

/build

Use one generic error status for rejected, missing, and partial client replies. Consolidate equivalent job-log security and failure cases without reducing coverage.
Comment thread nvflare/private/fed/server/job_cmds.py Outdated
Append one generic error item so both command metadata consumers and the legacy interactive console observe client rejection, timeout, and missing-response failures.
@pcnudde

pcnudde commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator Author

/build

@pcnudde

pcnudde commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile-apps The remaining 4/5 concern is specific to the legacy interactive HCI status classifier. That classifier is outside the scope of this minimal 2.8 security backport. The relevant Session API path consumes the error metadata and correctly reports client configuration failures, while the appended error is also visible in HCI output. We intentionally do not want to broaden this backport by changing generic HCI status-classification behavior. Please reevaluate the PR with that scope.

@pcnudde
pcnudde requested a review from nvidianz August 25, 2026 22:28
@YuanTingHsieh

Copy link
Copy Markdown
Collaborator

Approved after local smoke testing on the exact PR head using a Python virtualenv.

  • Default one-client PocEnv reached readiness, the client registered successfully, and graceful shutdown completed.
  • A PocEnv explicitly configured with a localhost server target bound both external and internal listeners to 127.0.0.1; the client connected and registered successfully.
  • 135 targeted unit tests passed.

Non-blocking notes:

  • Remote job logging now accepts only log levels and built-in log modes; dictConfig, inline JSON, and file paths are intentionally rejected as part of the security hardening.
  • The default PocEnv still generates a server:8002 server target, so its external listener retains the pre-existing wildcard bind. The new loopback behavior activates when the server target is explicitly localhost.

@chesterxgchen
chesterxgchen merged commit e123603 into NVIDIA:2.8 Aug 25, 2026
27 checks passed
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.

3 participants