draft: add package-owned gateway isolation controls - #26
Closed
MythiliMur wants to merge 1 commit into
Closed
MythiliMur wants to merge 1 commit into
MythiliMur wants to merge 1 commit into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| public GatewayIsolationState Read() | ||
| { | ||
| if (!File.Exists(_path)) |
| try | ||
| { | ||
| return JsonSerializer.Deserialize<GatewayIsolationState>( | ||
| File.ReadAllText(_path)) ?? |
| { | ||
| string directory = Path.GetDirectoryName(_path) ?? | ||
| throw new InvalidOperationException("The isolation state path has no directory."); | ||
| Directory.CreateDirectory(directory); |
| string temporary = Path.Combine(directory, $".{FileName}.{Guid.NewGuid():N}.tmp"); | ||
| try | ||
| { | ||
| File.WriteAllText(temporary, JsonSerializer.Serialize(state)); |
| } | ||
| finally | ||
| { | ||
| if (File.Exists(temporary)) |
| { | ||
| if (File.Exists(temporary)) | ||
| { | ||
| File.Delete(temporary); |
clawsweeper
Bot
added
P2
|
Codex review: blocked before merge. Reviewed September 10, 2026, 7:17 PM ET / 23:17 UTC. ClawSweeper reviewWhat this changesAdds package-owned commands to save and display a requested Gateway isolation setting, plus a launch guard for explicitly supplied enabled state. Merge readiness⛔ Blocked before merge - 11 items remain Keep open as a draft: current main does not provide these controls, and the contribution has distinct value, but two concrete defects prevent safe use. Collaborator-authored work also requires explicit maintainer handling. Priority: P2 Review scores
Verification
How this fits togetherThe Windows package exposes clawctl for package management and openclaw for launching the bundled CLI through device-installed Node.js. This change stores an isolation preference beside launcher data, but the production process-launch path does not consume it. flowchart TD
A[clawctl isolation command] --> B[Saved isolation preference]
B --> C[Requested status output]
D[openclaw command] --> E[Production launcher]
E --> F[Node process in interactive session]
G[Explicit isolation argument] --> H[Unsupported isolation guard]
Decision needed
Why: The body deliberately defers the security boundary, so deciding what users may safely be offered requires agreement on the staged product contract. Before merge
Findings
Agent review detailsSecurityNeeds attention: The requested isolation posture is not enforced at process creation; no additional supply-chain or reachable path-traversal defect was established. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep isolation explicitly unavailable until a verified Windows boundary exists, with NativeAOT-safe preferences enforced before process creation and preserved across package upgrades. Do we have a high-confidence way to reproduce the issue? Yes, source establishes both failure paths: startup ignores saved enabled state, and the new JSON overloads conflict with the package's NativeAOT contract. No runtime execution was performed. Is this the best way to solve the issue? Unclear as a landing strategy: package ownership is consistent with the existing launcher boundary, but shipping public enable controls before enforcement exists requires a narrower, explicit contract. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against 51969c7b2022. LabelsLabel changes:
Label justifications:
EvidenceSecurity concerns:
What I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
Collaborator
Author
|
This is being handled here - #28 |
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
2 participants
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
Adds a package-owned
clawctl gateway-isolationcontrol surface:statusreports the requested next-launch posture;enableanddisableatomically persist a package-local requested state;Deliberately draft / not merge-ready
This change does not pretend an environment marker is isolation. The launcher rejects an enabled isolation request until a real Windows isolation boundary exists. The follow-up must create and verify that boundary (for example, the planned dedicated isolated agent session), then use this state as its source of truth.
The paired OpenClaw core proposal should remain platform-neutral: an authenticated, external-supervisor control bridge so Control UI can modify a launcher-owned setting without core hard-coding Windows isolation semantics.
Validation
dotnet test .\OpenClaw.Gateway.MSIX.slnx --configuration Release --no-restore