Fix isolation error wording and log-level rationale from #5794 review#5853
Merged
Conversation
Address post-merge review feedback on #5794: the fail-fast error for --isolate-network conflicting with a non-bridge network mode assumed the mode came from --network, but it can also come from a --permission-profile file, making the suggested "drop --network X" remedy inactionable. The error now names the resolved mode directly. Also clarify why the "none" and host/custom degrade paths log at different levels in ReadJSON. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
aponcedeleonch
requested review from
JAORMX,
amirejaz and
lujunsan
as code owners
July 17, 2026 12:56
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5853 +/- ##
==========================================
+ Coverage 71.16% 71.23% +0.06%
==========================================
Files 689 689
Lines 70110 70110
==========================================
+ Hits 49896 49941 +45
+ Misses 16595 16533 -62
- Partials 3619 3636 +17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
rdimitrov
approved these changes
Jul 20, 2026
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.
Summary
#5794 (merged) reconciled network isolation with network mode. rdimitrov's
post-merge review left two follow-up comments this PR addresses:
--isolate-network=trueconflicting with anon-bridge network mode assumed the mode always came from
--network,telling users to
drop --network hosteven when the mode actually camefrom a
--permission-profilefile (where there's no--networkflag todrop). The error now names the resolved mode directly instead of assuming
a specific flag.
degradeNetworkIsolationwhy thenoneand host/customdegrade paths intentionally log at different levels:
noneis DEBUG(redundant, not a security reduction), while host/custom stays at WARN
even on read paths (
thv list, status, export) since it means theuser's
--isolate-networkrequest is being silently ignored.Type of change
Test plan
task test)task lint-fix)Updated
TestRunConfigBuilder_NetworkIsolationReconciliationinpkg/runner/config_builder_test.goto assert the error names the resolvedmode instead of an assumed
--networkflag.Does this introduce a user-facing change?
Yes. The fail-fast error message when
--isolate-network=trueconflictswith a non-bridge network mode now names the resolved network mode
directly (e.g.
"host") instead of assuming it came from--network.🤖 Generated with Claude Code