feat(framework): Add --root-certificates argument for flwr-* commands and flower-superexec#6986
feat(framework): Add --root-certificates argument for flwr-* commands and flower-superexec#6986danieljanes merged 24 commits intomainfrom
--root-certificates argument for flwr-* commands and flower-superexec#6986Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39d56b39b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if insecure or root_cert_path is None: | ||
| return None |
There was a problem hiding this comment.
Returning None when root_cert_path is omitted conflates two different states: secure TLS with system CAs and fully insecure transport. The new callers in flwr_clientapp, flwr_serverapp, and flwr_simulation pass this value as certificates, and their runtime connections compute insecure=(certificates is None), so --insecure off plus no --root-certificates now opens a plaintext channel instead of TLS (and fails against TLS-only AppIO endpoints). This is a silent security downgrade introduced by enabling these code paths without preserving the explicit insecure/secure signal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR introduces a --root-certificates CLI flag to enable TLS (with custom roots) for AppIO-style gRPC connections used by flwr-* executor commands and flower-superexec, and adds supporting exit codes/docs/tests.
Changes:
- Add
--root-certificatesto commonflwr-*app CLI args and toflower-superexec. - Introduce
flwr.supercore.tls.load_root_certificatesplus new exit codes/docs for TLS flag conflicts and invalid paths. - Wire root certificate loading through SuperExec and executor processes, with added unit tests.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| framework/py/flwr/supernode/cli/flwr_clientapp.py | Uses load_root_certificates and forwards cert bytes into run_clientapp. |
| framework/py/flwr/supercore/tls.py | Adds helper to validate/resolve root certificate bytes and enforce flag compatibility. |
| framework/py/flwr/supercore/tls_test.py | Adds tests for the TLS helper behavior and error paths. |
| framework/py/flwr/supercore/superexec/run_superexec.py | Adds TLS parameters to SuperExec channel creation and forwards TLS config into plugins. |
| framework/py/flwr/supercore/superexec/plugin/exec_plugin.py | Extends plugin base class to retain TLS-related settings. |
| framework/py/flwr/supercore/superexec/plugin/base_exec_plugin.py | Forwards --insecure or --root-certificates when spawning executor subprocesses. |
| framework/py/flwr/supercore/superexec/plugin/base_exec_plugin_test.py | Adds coverage for forwarding/omitting TLS flags in subprocess args. |
| framework/py/flwr/supercore/cli/flower_superexec.py | Adds --root-certificates CLI arg and forwards it into run_superexec. |
| framework/py/flwr/simulation/app.py | Loads root certs and passes them into the Simulation AppIO connection wiring. |
| framework/py/flwr/simulation/app_test.py | Adds parsing test for --root-certificates and verifies certs passed into connection. |
| framework/py/flwr/server/serverapp/app.py | Loads root certs and passes them into the ServerApp gRPC grid wiring. |
| framework/py/flwr/common/exit/exit_code.py | Adds new exit codes 603/604 and help text. |
| framework/py/flwr/common/args.py | Adds --root-certificates to shared executor CLI args. |
| framework/docs/source/ref-exit-codes/603.rst | Documents new exit code for incompatible TLS flags. |
| framework/docs/source/ref-exit-codes/604.rst | Documents new exit code for invalid path arguments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf0e4a8179
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc99645923
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
github-actions
Bot
added
the
Maintainer
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b3fc32a1f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6852c8a898
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f19ee028af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
msheller
left a comment
msheller
left a comment
There was a problem hiding this comment.
LGTM! Just small text/naming suggestions.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acc1e95c63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c1f758ad1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6eda1e3981
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| clientappio_api_address=args.clientappio_api_address, | ||
| token=args.token, | ||
| certificates=None, | ||
| insecure=args.insecure, |
There was a problem hiding this comment.
Forwarding insecure=args.insecure here makes the secure path reachable for flwr-clientapp, but run_clientapp still catches grpc.RpcError and then exits with ExitCode.SUCCESS unconditionally (framework/py/flwr/supernode/runtime/run_clientapp.py). On deployments where ClientAppIo is still plain gRPC (for example, SuperNode currently starts it with certificates=None), omitting --insecure now produces TLS/RPC failures that are reported as a successful run, which can silently break automation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d4c6ab05a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| insecure=args.insecure, | ||
| certificates=validate_and_resolve_root_certificates( | ||
| args.root_certificates, args.insecure | ||
| ), |
There was a problem hiding this comment.
This change enables secure mode by default for flwr-serverapp/flwr-simulation (--insecure defaults to false), but the SuperLink still starts ServerAppIo with certificates=None (framework/py/flwr/server/app.py, run_serverappio_api_grpc call), so these runners will now attempt TLS against a plaintext endpoint and fail the connection handshake. Previously this path was blocked with COMMON_TLS_NOT_SUPPORTED; now default secure invocations fail later and cannot succeed until server-side TLS is actually wired for ServerAppIo.
Useful? React with 👍 / 👎.
No description provided.