feat: macOS menubar app that provides network ingress/egress filtering and better UX#60
feat: macOS menubar app that provides network ingress/egress filtering and better UX#60
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5cc6b65b6
ℹ️ 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 queryUserServiceLoaded() { | ||
| _ = try runCommand("/bin/launchctl", ["kickstart", "-k", serviceTarget]) | ||
| return |
There was a problem hiding this comment.
Re-bootstrap launch agent after updating its plist
startOrRestartUserService always rewrites the LaunchAgent plist, but when the job is already loaded this branch only runs launchctl kickstart -k and returns. That means changes in the plist (notably ProgramArguments when Cleanroom.app moves or is reinstalled, and KeepAlive/RunAtLoad toggles) are not reloaded into launchd, so the service can keep using stale settings or fail to restart after an app path change. Reloading with bootout + bootstrap when the plist is updated avoids this stale-config path.
Useful? React with 👍 / 👎.
|
@codex review please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0248d295d
ℹ️ 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".
| tempPath := path + ".tmp" | ||
| if err := os.WriteFile(tempPath, data, 0o644); err != nil { | ||
| return fmt.Errorf("write network filter policy snapshot temp file: %w", err) | ||
| } | ||
| if err := os.Rename(tempPath, path); err != nil { |
There was a problem hiding this comment.
Use per-write temp files for policy snapshot updates
writeNetworkFilterPolicySnapshot always writes through the fixed path path + ".tmp", but SyncNetworkFilterPolicy() is invoked from multiple execution lifecycle paths and can run concurrently. Two overlapping syncs can overwrite each other’s temp file and cause one os.Rename to fail (or publish stale content), leaving outdated allow/process_rules in place until a later sync, which can make host filtering inconsistent with currently running executions.
Useful? React with 👍 / 👎.
| func evaluateNetworkPolicyForDoctor(networkDefault string, allowCount int, allowlistSupported bool) (string, error) { | ||
| return evaluateNetworkPolicy(networkDefault, allowCount, allowlistSupported, false) |
There was a problem hiding this comment.
Enforce allowlist availability in doctor policy checks
evaluateNetworkPolicyForDoctor explicitly disables allowlist enforcement, so when sandbox.network.allow is present and host filtering is unavailable, doctor only warns even though the run path rejects that same configuration as an error. This creates a misleading diagnostic path where cleanroom doctor can appear healthy but execution still fails immediately for policy reasons.
Useful? React with 👍 / 👎.
f0248d2 to
c2f6957
Compare
Summary
macos/main.swift) with a one-clickEnable Cleanroomonboarding flowcleanroom serve, with advanced controls for restart/stop and optional system daemon installRun Server At Logintoggle persisted in app preferences and reflected in generated LaunchAgent plistCleanroom.app:cleanroom,cleanroom-darwin-vz, linux guest-agent artifact, app icon + menubar template iconsdocs/macos-app.mdTesting
mise exec go -- go test ./internal/backend/darwinvz ./internal/cli -run 'Test|ServeInstall|ServeCommand|DaemonRunArgs'mise run build:macos-appmise run install:macos-appmise run lint-shellcleanroom doctorreports 0 fail forhelper_binary/guest_agent_binaryNotes
macos/app-icon.pxd(source design artifact); PNG assets used by build are committed