Pass Softnet policy control FD through Tart - #1287
Conversation
|
|
||
| // Automatically enable --net-softnet when any of its related options are specified | ||
| if netSoftnetAllow != nil || netSoftnetBlock != nil || netSoftnetExpose != nil { | ||
| if netSoftnetAllow != nil || netSoftnetBlock != nil || netSoftnetExpose != nil || (netSoftnetControlFd != nil && !netHost) { |
There was a problem hiding this comment.
Why special-case netSoftnetControlFd?
Like the other Softnet-related options, it should enable netSoftnet, allowing the existing validation to reject its use with netHost:
diff --git a/Sources/tart/Commands/Run.swift b/Sources/tart/Commands/Run.swift
@@
- The file descriptor must be greater than 2. Implies --net-softnet unless --net-host is specified.
+ The file descriptor must be greater than 2. Implies --net-softnet.
@@
- if netSoftnetAllow != nil || netSoftnetBlock != nil || netSoftnetExpose != nil || (netSoftnetControlFd != nil && !netHost) {
+ if netSoftnetAllow != nil || netSoftnetBlock != nil || netSoftnetExpose != nil || netSoftnetControlFd != nil {
netSoftnet = true
}
diff --git a/Tests/TartTests/SoftnetControlFDTests.swift b/Tests/TartTests/SoftnetControlFDTests.swift
@@
- func testControlFDWorksWithHostNetworking() throws {
+ func testControlFDIsRejectedWithHostNetworking() throws {
let temporaryHome = try createTemporaryTartHome()
defer { try? FileManager.default.removeItem(at: temporaryHome) }
let previousHome = ProcessInfo.processInfo.environment["TART_HOME"]
setenv("TART_HOME", temporaryHome.path, 1)
defer { restoreEnvironment("TART_HOME", value: previousHome) }
- let command = try Run.parse(["vm", "--net-host", "--net-softnet-control-fd", "3"])
-
- XCTAssertTrue(command.netHost)
- XCTAssertFalse(command.netSoftnet)
- XCTAssertEqual(command.netSoftnetControlFd, 3)
+ XCTAssertThrowsError(
+ try Run.parse(["vm", "--net-host", "--net-softnet-control-fd", "3"])
+ )
}There was a problem hiding this comment.
Addressed in 311eec9. --net-softnet-control-fd now consistently implies --net-softnet, so combining it with --net-host is rejected by the existing mutual-exclusion validation. Updated the help text and test accordingly. Full Swift test suite passes (56 tests).
|
|
||
| init(vmMACAddress: String, extraArguments: [String] = []) throws { | ||
| init(vmMACAddress: String, extraArguments: [String] = [], controlFD: Int32? = nil) throws { | ||
| if let controlFD = controlFD { |
There was a problem hiding this comment.
Could we use an owning FileHandle with closeOnDealloc: true here?
That would remove the manual closeControlFD()/deinit bookkeeping while still allowing run() to close the parent’s copy after launching Softnet.
There was a problem hiding this comment.
Addressed in 4a018ce. The control socket now uses an owning FileHandle(closeOnDealloc: true); run() explicitly closes the parent copy after launch, and the raw-FD/deinit bookkeeping is gone. Standard descriptors are rejected before ownership is taken, with regression coverage to ensure stderr remains open. Full Swift test suite and SwiftFormat lint pass.
tart 2.34's softnet integration closes its own stdout right after spawning softnet (openai/tart#1287: Softnet.run()'s defer closes an unset Process.standardOutput, which Foundation resolves to the parent's fd 1). Measured on this host: with the softnet flags present the run log stays 0 bytes and lsof shows fd 1 replaced by a kqueue; the identical command minus the three --net-softnet* flags writes "VNC server is running at <url>" to the same log within seconds. The URL -- the only place the per-boot VNC password ever exists -- is unrecoverable host-side, so the prior behaviour was a guaranteed 90 s wait ending in a fail-closed stop of a VM that booted for nothing. Refuse before `tart run`, keyed on darwin x vnc x a --net-softnet* flag in the policy x tart 2.34.*: pre-2.34 lacks the regression, --net-bridged never touches the broken path, and on any newer release the 90 s URL wait below still fails closed. Mutation-checked: deleting the guard turns the refusal cases red (3 failed) while all four controls stay green. Assisted-by: Claude Code:claude-fable-5
Summary
Passes a dedicated Softnet policy-control socket through
tart run, enabling callers such as Orchard to dynamically update allow/block lists while leaving the existing VM packet socket unchanged. The forwarded protocol is intentionally limited tosoftnet.policy.getandsoftnet.policy.set.Depends on openai/softnet#181.
Changes
--net-softnet-control-fdand imply Softnet networking when appropriate (including host-network support)--control-fd 1; stdin remains the existing Unix datagram packet socketsoftnet.policy.setterminologyValidation
SoftnetControlFDTestspassed in a clean exportgit diff --checkThe clean export intentionally excludes unrelated, pre-existing local Windows/QEMU files in the checkout.