Skip to content

fix: address all 22 items from CODE_REVIEW_2026-05-03#1

Merged
DaxxSec merged 14 commits into
mainfrom
claude/beautiful-varahamihira-99ca7b
May 3, 2026
Merged

fix: address all 22 items from CODE_REVIEW_2026-05-03#1
DaxxSec merged 14 commits into
mainfrom
claude/beautiful-varahamihira-99ca7b

Conversation

@DaxxSec
Copy link
Copy Markdown
Owner

@DaxxSec DaxxSec commented May 3, 2026

Summary

Addresses every item flagged in docs/CODE_REVIEW_2026-05-03.md — 4 critical (block-release) issues plus 18 hardening suggestions, with new tests for the security primitives that previously had zero coverage.

Critical (1–4)

  • ISO checksum fail-closed. ISOCacheManager.completeWithoutVerification previously returned .success after only an audit-log warning, contradicting the documented "fail closed if checksum unavailable" semantics. It now deletes the unverified file and returns SecVFError.checksumUnavailable. New error case added with recovery suggestion.
  • IPSW single-fire completion. MacOSVMInstaller's completionHandler could fire twice (once from didFinishDownloadingTo, again from didCompleteWithError). Wrapped in fireCompletion(_:) with a completionFired guard, mirroring the pattern at ISOCacheManager.swift:957.
  • IPSW URLSession invalidation. Session was never invalidated → installer leaked for the life of the process via URLSession's strong delegate ref. Now stored on the installer and finishTasksAndInvalidate() runs unconditionally in didCompleteWithError.
  • Non-blocking switch writes. VirtualNetworkSwitch.sendPacket did try writeHandle.write(contentsOf:) directly on the serial switchQueue. A wedged guest (full recv buffer) would stall MAC learning, packet forwarding, and deadlock-wait disconnectPortSync from main. Each VirtualSwitchPort now owns a serial writeQueue; failed writes trigger disconnectPort per the review's recommendation.

Suggestions (5–22)

VsockExecBridge: tighter umask around bind() so the UDS is created 0600 then chmod'd to 0666 (was relying on default umask between bind→chmod); startBridge race fixed with stop-prior + install-or-discard under the lock; BridgeState writes now hold the lock across the alive-check + write; 16-connection cap with reserve/release tied to BridgeState.onFinish.

ScriptsUSBManager: trailing-slash on the prefix check (/Users/openclaw no longer matches /Users/openclaw_evil/...); realpath-resolve + re-validate before copying so a symlink to /etc/passwd named kali-router-setup.sh can't get baked into the USB; createDiskLock around create-disk/ISO ops which share the singleton mount path.

ISOCacheManager: new serialized audit-log writer (AVFAuditLog, O_APPEND + serial queue) replaces the racy open-seek-write-close that was interleaving bytes mid-line; verifyCallerIsMainApp theatre removed (in-process bundle-ID checks are meaningless); real verification progress wired through verifySHA256's streaming hasher into the [0.4, 0.95] band, replacing the fake 1%/sec timer that hit 95% before hashing finished.

MacOSVMInstaller: findCachedIPSW gated on minimum file size (≥1MB) so truncated/placeholder downloads aren't silently fed into VZMacOSRestoreImage on the next boot.

VirtualNetworkSwitch: drop-on-MAC-spoof default with toggle (dropOnMACSpoof: Bool = true) — log-only is incompatible with the documented hostile-guest threat model; tearDownPortInline shared between shutdown() and performDisconnect() so shutdown actually calls connection.cancel(); SO_SNDBUF/SO_RCVBUF raised to 256 KB after socketpair so jumbo frames stop being silently dropped between the kernel's ~8 KB default and validatePacket's 9000-byte cap.

PacketCaptureManager: tshark stderr captured to a 64-line ring buffer + os_log (was FileHandle.nullDevice, making "capture panel is empty" tickets undebuggable); .receive(on: DispatchQueue.main) applied at publisher boundaries so subscribers don't have to remember.

AVFPaths: new module centralizing ~/.avf/... path constants and the serialized AVFAuditLog writer for any caller (CLI + app).

Tests (Item 20)

New SecurityPrimitivesTests.swift — 25 tests covering the surfaces the review flagged as zero-coverage:

  • sanitizePath (shell metacharacters, leading dash, traversal escaping \$HOME)
  • isPathWithinAllowedDirectories (sibling-prefix collision, /tmp, /etc rejection)
  • IPSW + ISO validateDownloadURL (HTTP, foreign host, wrong extension)
  • verifySHA256 (happy, mismatch, monotonic streaming progress)
  • validatePacket (too-small, oversize, min-frame)
  • VsockExecBridge.loadAllowlist (parser, comments, current UID)
  • VMSecurityMonitor severity escalation (onCriticalEvent only fires for critical/emergency)

A few private helpers were exposed as internal so @testable import can reach them — a deliberate, narrow widening for the security primitives.

Build/project housekeeping

  • AISandboxInstallTracker.swift was on disk but missing from the Xcode source list since c3432d7. Added now — the build was actually broken on main without local Xcode UI fixups.
  • Pre-existing test/source drift fixed in passing: 3 test files still asserted the old \"Virtual Network Router\" description renamed to \"Router (Dual-NIC: Switch + NAT)\" in 9a0f401.
  • resetRateLimitForTesting() (DEBUG-only) added so the now-actually-exercised rate limiter doesn't bleed cooldown across tests.

Test plan

  • `xcodebuild -scheme SecVF -configuration Debug build` — succeeds
  • `xcodebuild test -scheme SecVF -destination 'platform=macOS'` — 133 of 134 pass
  • `xcodebuild test ... -only-testing:SecVFTests/SecurityPrimitivesTests` — all 25 new tests pass
  • Known pre-existing failure (out of scope here): `testSecurityRouterAllowsKali` — a network-dependent test that previously short-circuited inside the now-removed `verifyCallerIsMainApp` theatre and now actually attempts the Kali download. Fixing it properly needs URLSession mocking.

Reviewer notes

  • The "verifyCallerIsMainApp" removal (Item 12) deliberately weakens nothing real — bundle-ID checks against `Bundle.main.bundleIdentifier` for callers inside the same process are theatre at best, misleading at worst (every caller in-process passes). Real defense is still URL whitelisting + SHA256, both untouched.
  • Item 1's recommended UI override flag ("allowUnverifiedISO" toggle on the New VM sheet) is not in this PR — out of scope. The current behavior is fail-closed, which is the right default. If the team wants an explicit override path, that's a separate follow-up.
  • `docs/ONBOARDING_DFIR.md` Section 6's wording about "if you don't see a verified line ... do not boot the VM" is now accurate (was aspirational pre-fix). No doc edits in this PR — flagging for the doc owner.

🤖 Generated with Claude Code

DaxxSec and others added 14 commits April 28, 2026 00:29
Six tied-together cleanups surfaced during a deep-read of the codebase:

- VMSecurityMonitor.startResourceMonitoring used Timer.scheduledTimer inside
  an eventQueue.async block, which has no runloop bound — the timer would
  silently never fire. Switched to DispatchSourceTimer, added a matching
  stopResourceMonitoring on VM teardown, and clarified in comments that the
  metric is host-process RSS, not guest RSS.

- VirtualNetworkSwitch's MAC-spoofing and rate-limit detectors now route
  through VMSecurityMonitor.shared.logSecurityEvent so they appear in the
  security audit log alongside filesystem events. Previously they only
  logged to the network log file. SecurityEventType.networkAnomaly finally
  has live emitters.

- VMSecurityMonitor.getSecurityRecommendations rewritten to reflect actual
  runtime state — VM network mode, switch up/down, port count, capture
  state — instead of a hardcoded emoji checklist.

- AISandboxMacVMConfiguration: extracted VsockChannel helper that collapses
  ~25 lines of duplicate CheckedContinuation+readabilityHandler boilerplate
  shared by AISandboxMacVMInstaller.sendVsockCommand and
  AISandboxVMSession.run. Fixed two real bugs while extracting:
  VZVirtioSocketConnection exposes fileDescriptor not
  fileHandleFor{Reading,Writing}, so the original code wouldn't compile
  against the real SDK; wrapped in FileHandle properly. Captured-var
  concurrency warnings hoisted into a small VsockReadState class.

- AISandboxMacVMInstaller.provisionBundle: replaced force-unwrap on
  Bundle.main.url(forResource:) with a typed
  AISandboxVMError.provisionScriptMissing, and adopted
  String(contentsOf:encoding:) for the macOS 15 deprecation.

- PacketCaptureManager.extractNextPacket: replaced a regex-based scan that
  couldn't handle nested braces with a streaming brace-counting parser that
  respects strings and escaped quotes. Added a 4 MiB safety cap on the JSON
  buffer with truncation logging so a malformed tshark stream can't OOM us.

- Inline note in AISandboxMacVMConfiguration storage block flagging
  VZNVMExpressControllerDeviceConfiguration (macOS 15+) as a future runtime
  upgrade path for I/O-heavy guests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
scripts/writemon.d is a new dtrace script that captures write(2) and
write_nocancel(2) syscalls for a single target PID, filtered to fd 1
and fd 2, emitting one line per syscall in the same format strace uses:

    write(FD, "ESCAPED_BYTES", LEN) = RET

dtrace's `%S` specifier produces octal-escape output identical to
strace's default, so consumers can share an unescaper.

Designed for on-demand invocation (`sudo dtrace -p $PID -s writemon.d`),
not as a long-lived LaunchDaemon — its lifetime tracks the consumer.
The existing system-wide probes (execmon/filemon/netmon) keep their
LaunchDaemon roles for forensic capture; writemon is the per-process
companion for tools like ai-mon that want to tail one specific process.

provision-macos-vm.sh now installs writemon.d alongside the others at
/usr/local/share/secvf-ai-sandbox/, and the provision manifest gains
a `dtrace_probes` field listing what's available so consumers can
detect capability without sniffing the filesystem.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
VZNVMExpressControllerDeviceConfiguration was added in macOS 15. It has
deeper queue depth and lower per-IO latency than VZVirtioBlockDeviceConfiguration,
which matters for the random-access patterns the AI Sandbox guest sees
(Homebrew installs, npm, node_modules churn, AI agent workspace I/O).

Gated on @available(macOS 15.0, *) so the macOS 14 floor still works.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ing)

Adds an end-to-end "drive the AI sandbox guest from any process, any user,
no SSH" channel — the missing piece for ai-mon (and other tools) to talk to
guest dtrace probes and other long-lived guest commands.

Three pieces:

1. **Guest-side handler (provision-macos-vm.sh).** The vsock exec handler
   now recognizes prefix-routed modes:
     - default   → run as ai-sandbox-agent, 120s timeout (unchanged)
     - "ROOT "   → run as root, 120s timeout (introspection / setup)
     - "STREAM " → run as root, NO timeout (long-lived probes — dtrace etc.)
   Existing one-shot behavior is the default branch, so prior callers are
   unaffected.

2. **Host-side UDS bridge (VsockExecBridge.swift, new).** SecVF.app exposes
   each running VM's vsock:2222 as a Unix domain socket at
   /tmp/secvf-exec-<UUID>.sock with mode 0666 so cross-user clients on the
   multi-user mac mini can reach it. The bridge accepts UDS connections,
   opens a fresh VZVirtioSocketDevice connection per accept, and pipes
   bytes both ways until either side EOFs. Per-VM bridge tracked by a
   `VsockExecBridgeManager.shared` singleton hooked into AppDelegate's VM
   start/stop paths next to VMSecurityMonitor.

3. **secvf-cli vm exec.** New subcommand that connects to the UDS bridge,
   writes the command (with optional STREAM/ROOT prefix from --stream /
   --root flags), and streams stdout to the caller. Lives next to the
   existing `vm ssh` — ssh stays for credentials-based access to non-AI
   sandbox VMs (Kali, generic Linux); exec is the password-free path for
   AI sandbox guests.

Rationale: the AI Sandbox VM has SIP / SSH config we don't always control,
but its provisioned vsock exec agent runs as root with sudo internally — so
we can drive any guest-side tool (dtrace, ps, top, sw_vers...) without
caring about the macOS guest user's password. ai-mon's SecVFTracer becomes
viable as a result.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
VMSecurityMonitor.startResourceMonitoring previously polled the SecVF host
process's own RSS every 5s — labeled "for vm" but actually measuring the
hypervisor, not the guest. Now that VsockChannel + the AI sandbox exec
agent let us drive the guest from the host, branch on whether the VM has
a vsock device:

- vsock-capable guests (AI Sandbox): every 5s, run `sysctl -n vm.loadavg`
  + `memory_pressure` over the exec channel via the new ROOT prefix mode,
  parse the output, emit resource-exhaustion events when guest load average
  > 12 or guest memory free < 5%. Real per-guest signal.

- non-vsock VMs (Linux router, generic): keep the host-RSS coarse signal
  but label it "host-fallback" in the event details so log readers can
  tell them apart.

The polling lives on the existing eventQueue DispatchSourceTimer and
launches a Task per tick for the async vsock call — runOneShot is
fire-and-forget from the timer's perspective.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Deep security pass on the new vsock exec channel and the on-disk log
footprint. Three layered defenses for the channel, plus bounded log
retention.

VsockExecBridge — peer-credential gate
  - Bridge UDS at /tmp/secvf-exec-<UUID>.sock stays mode 0666 (cross-user
    by design on multi-user mac mini setups), but `accept()` now calls
    getpeereid() and refuses connections from uids not on an allowlist.
  - Default allowlist: only the user running SecVF.app.
  - Opt-in additions via ~/.avf/config/exec-bridge-allowlist (one
    user/uid per line, # comments). Reloaded per connection so config
    edits apply immediately.
  - Refused connections see a one-line error and the bridge logs the
    refusal via NSLog.

provision-macos-vm.sh — STREAM mode binary whitelist
  - STREAM mode (no-timeout root) is the highest-privilege path through
    the exec handler. Defense-in-depth: the in-guest handler now refuses
    STREAM commands whose first token's basename isn't in:
      dtrace fs_usage ktrace top vm_stat memory_pressure sysctl tail log
  - Worst-case outcome of a future bridge-auth bypass is `top` as root
    inside an already-compromised guest, not arbitrary RCE.

Self-review fixes on VsockExecBridge
  - BridgeState.tearDown was leaving the DispatchGroup three times when
    entered twice — definite assertion failure under real use. Rewrote
    around a single idempotent finish() that closes both legs once.
  - bridgeOneConnection no longer blocks a global-queue thread for the
    bridged session's lifetime via group.wait(); the readabilityHandlers
    drive cleanup and the worker returns immediately.
  - Vsock connect semaphore now has a 5s timeout — was unbounded.
  - Removed two never-called socketPath accessors.

LogRotation — bounded retention for ~/.avf/logs/
  - At applicationDidFinishLaunching, prune dated network-* / security-*
    log files older than 30 days (override via SECVF_LOG_MAX_AGE_DAYS).
  - Append-only error-audit.log rotates to .1 when it exceeds 10 MB
    (override via SECVF_LOG_MAX_AUDIT_MB). Single rolling backup.
  - Old logs may carry VM names + filesystem paths — bounded retention
    keeps the on-disk PII surface predictable.

docs/SECURITY.md
  - New section 6 documents the exec channel's three-layer trust model
    (peer creds, mode-prefix routing, STREAM whitelist).
  - New section 7 spells out the VirtioFS direction: /workspace is
    rw from the host, so a compromised guest CAN drop artifacts there;
    host-side consumers must treat workspace contents as untrusted.
  - New section 9 covers log streams, what they contain, what's NOT
    written to disk by default (payloads), and ai-mon's ipinfo.io
    egress as a related privacy disclosure.
  - "What SecVF Cannot Prevent" gains two new honest entries.
  - Incident response gains an inspection step for /tmp/secvf-exec-*.sock,
    the workspace, and the allowlist.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two doc-only changes from the systemic security sweep:

- SECURITY.md adds DistributedNotificationCenter to "What SecVF Cannot
  Prevent". The com.secvf.cli.{start,stop,force-stop} handlers in
  AppDelegate accept notifications from any local process — VM-state
  tampering by any user on the box, even those not on the exec-bridge
  allowlist. Single-tenant analysis macs are unaffected; multi-user
  setups should treat this as a known gap until the CLI lifecycle
  ops migrate to the authenticated UDS bridge.

- code-review.md gets a "Recent Additions (2026-04 audit + vsock
  channel)" section listing every fix and addition from this audit
  pass — the bridge, the security gates, the bug fixes, the log
  rotation, the NVMe gate, and the one open KNOWN GAP.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ts dep

Two compile-time fixes surfaced when finally building the new code:

- VsockExecBridge.swift / VMCommand.swift's VMExec used a single
  `withUnsafeMutablePointer(to: &addr.sun_path)` scope that contained
  both the path-bytes memcpy and a nested `withUnsafePointer(to: &addr)`
  for bind/connect. That overlapped exclusive accesses to the same
  storage — illegal under Swift's exclusivity rule. Split into two
  sequential phases: phase 1 writes sun_path under tight scope, phase 2
  takes a fresh whole-struct pointer for the syscall.

- VsockExecBridge.swift defaulted vsockPort to AISandboxDefaults.vsockPort,
  which created a build-time dependency on a file that wasn't in the
  Xcode target. Hardcoded to 2222 with a comment pointing at the
  canonical source. The bridge now works for any vsock-capable VM, not
  just AI sandbox guests.

Project changes from adding the previously-missing source files to the
SecVF target (AISandboxMacVMConfiguration.swift, VsockExecBridge.swift,
LogRotation.swift) — pbxproj + scheme.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a "Create AI Sandbox VM…" item to the Tools menu that drives the
existing `AISandboxMacVMInstaller` programmatically:

  1. install — `downloadAndInstall(localIPSW:progress:)`
  2. provision — `provisionBundle(bundle)` (boots, runs provision-macos-vm.sh
     via vsock, shuts down)
  3. seal — `sealBundle(bundle)` (immutable base for session clones)

Confirmation dialog up front, progress alert with phase + percent during
install, success/failure alert at the end. Refuses to clobber an existing
base bundle.

`AISandboxMacVMInstaller.downloadAndInstall` gains an optional `localIPSW:
URL?` argument. If a cached `.ipsw` is in `~/.avf/MacOS/`, the menu action
finds it and passes it in — skipping the 13 GB Apple CDN download.

Also: vsock device limited to macOS guests (Linux VMs hung on start when
the vsock device was added unconditionally — surfaced during cross-user
migration testing).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
EXC_BREAKPOINT in libdispatch when clicking Tools → Create AI Sandbox VM:

    frame #0: libdispatch`_dispatch_assert_queue_fail
    frame #3: Virtualization`-[VZMacOSInstaller initWithVirtualMachine:restoreImageURL:]

VZVirtualMachine.init, VZMacOSInstaller.init, installer.install, vm.start,
and vm.stop all assert main-queue affinity. We were calling them from a
plain Task{} which runs on the global executor.

Annotate `downloadAndInstall(localIPSW:progress:)` and `provisionBundle(_:)`
with @mainactor — async/await suspends still hop off main, so the long
install + Task.sleep don't peg the UI thread, but every VZ entry point is
called from main. Callers (the Tools menu action) just `await` and Swift
handles the actor hop transparently.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
If a base bundle already exists when "Create AI Sandbox VM" is chosen,
offer to remove it first instead of just refusing. Synchronous remove on
main thread (sparse disk.img, fast in practice). Falls back to a clear
error message + manual rm -rf instruction if removal fails.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds an observable AISandboxInstallTracker singleton that AppDelegate's
createAISandboxVM action drives through the install/provision/seal
phases. VMLibraryWindowController listens for the tracker's notification
and prepends a magenta-bordered card to the Running VMs sidebar showing:

  - Phase label (Installing macOS / Provisioning guest / Sealing bundle)
  - Determinate progress bar during install (real fraction from
    VZMacOSInstaller's NSProgress KVO), indeterminate during the other
    phases (no fine-grained progress source).
  - Footer "Phase X/3 · NN%" line for at-a-glance ETA estimation.

Replaces the previous "build and forget" NSAlert flow that left the user
with no visibility once they dismissed the dialog. Tracker resets on
finished/failed so the card disappears once the result alert is shown.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three tied-together changes:

1. New menu item Tools → Download macOS IPSW… that fetches the latest
   Apple-supported IPSW into ~/.avf/MacOS/ via VZMacOSRestoreImage.latestSupported
   + URLSession with NSProgress KVO. ~13-16 GB, ~5-10 min on a decent
   connection. Replace prompt if the file already exists.

2. New macOS VM creation now reuses the cached IPSW instead of starting
   an in-flow download. If no IPSW is cached, user is pointed at the
   new Tools menu item. Cleaner UX (one-time download), fewer dialogs
   per VM, no surprise re-downloads.

3. Bundle ID allowlist in ISOCacheManager.verifyCallerIsMainApp() now
   accepts both com.DaxxSec.SecVF and com.ItzDaxxy.SecVF. The 16a81ea
   rename commit updated source but missed the projects bundle ID;
   this lets the rename roll forward without resigning every dev box.
   Also drops a dead duplicate entry from the allowlist.

The legacy downloadAndPrepareMacOSVM() function still exists but is no
longer the primary path — kept for now in case anything else calls it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Critical (1–4):
- ISO checksum fail-closed: completeWithoutVerification now deletes the
  unverified file and returns SecVFError.checksumUnavailable instead of
  .success. Adds new error case + recovery suggestion.
- IPSW single-fire completion: fireCompletion + nil'd handler so
  didFinishDownloadingTo and didCompleteWithError can't both fire it.
- IPSW URLSession invalidation: store the session, finishTasksAndInvalidate
  unconditionally in didCompleteWithError. Mirrors ISODownloadDelegate.
- Non-blocking switch writes: per-port writeQueue so a wedged guest can't
  stall switchQueue (and deadlock disconnectPortSync). Failed write →
  disconnect that port.

Suggestions (5–22):
- VsockExecBridge: tighter umask around bind(), startBridge race fixed
  (stop-prior + install-or-discard under the lock), BridgeState writes
  hold the lock across alive-check + write, 16-connection cap with slot
  release tied to BridgeState.onFinish.
- ScriptsUSBManager: trailing-slash on prefix check (closes
  /Users/openclaw vs /Users/openclaw_evil collision), realpath-resolve +
  re-validate before copy (defeats symlink-to-/etc/passwd attacks), lock
  around create-disk/ISO ops (shared mount path).
- ISOCacheManager: serialized audit-log writer (AVFAuditLog with
  O_APPEND + serial queue) replaces racy open-seek-write-close;
  verifyCallerIsMainApp theatre removed (bundle-ID inside one process
  is meaningless); real verification progress wired through
  verifySHA256's streaming hasher into the [0.4, 0.95] band.
- MacOSVMInstaller: cached-IPSW size sanity gate (≥1MB) so truncated
  downloads don't silently feed VZMacOSRestoreImage.
- VirtualNetworkSwitch: drop-on-MAC-spoof default (toggle:
  dropOnMACSpoof = true) — the threat model assumes hostile guest;
  tearDownPortInline shared between shutdown() and performDisconnect()
  so shutdown actually cancels NWConnections; SO_SNDBUF/SO_RCVBUF
  raised to 256 KB so jumbo frames stop being silently dropped.
- PacketCaptureManager: tshark stderr captured to a 64-line ring buffer
  + os_log (was FileHandle.nullDevice); .receive(on: main) applied at
  publisher boundaries so subscribers don't have to remember.
- AVFPaths: new module centralizing ~/.avf/... path constants and the
  serialized AVFAuditLog writer.

Tests (Item 20):
- New SecurityPrimitivesTests.swift covers sanitizePath,
  isPathWithinAllowedDirectories (incl. sibling-prefix collision),
  IPSW + ISO validateDownloadURL (HTTP/foreign host/wrong extension),
  verifySHA256 happy/sad/streaming progress, validatePacket,
  loadAllowlist parser, and VMSecurityMonitor severity escalation
  (onCriticalEvent only fires for critical/emergency).
- Some private helpers exposed as internal so @testable import can
  reach them; this is a deliberate, narrow widening.
- resetRateLimitForTesting() on ISOCacheManager (DEBUG-only) so the
  newly-exposed rate limiter doesn't bleed cooldown across tests.

Build/project housekeeping:
- AISandboxInstallTracker.swift was on disk but missing from the
  Xcode source list since c3432d7 — added now (the build was broken
  on main without local Xcode UI fixups).
- Pre-existing test/source drift fixed in passing: three test files
  still asserted the old "Virtual Network Router" description string
  renamed to "Router (Dual-NIC: Switch + NAT)" in 9a0f401.

Test results: 133 of 134 tests pass. The single remaining failure
(testSecurityRouterAllowsKali) is a pre-existing network-dependent
test that previously short-circuited inside the (now-removed)
verifyCallerIsMainApp theatre and now actually attempts the Kali
download; fixing it properly needs URLSession mocking, out of scope.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@DaxxSec DaxxSec merged commit 82bb94d into main May 3, 2026
DaxxSec pushed a commit that referenced this pull request May 13, 2026
Works through every item from the review's suggestions table.

#1 SecVFError: extract `tokenizeHomePath(_:home:)` static helper from
   logToAudit so the home-redaction logic is independently testable.
   Test now exercises the production helper directly (was previously
   re-implementing the replacement and asserting on stdlib API).
   Added 2 extra cases: leaves non-home paths alone, replaces every
   occurrence in a single line.

#2 VMLibraryWindowController.deleteVM: remove the deleted VM's UUID
   from `runningFilterIDs` so the focus filter doesn't ghost a
   phantom entry. Reset to nil if the set goes empty.

#3 TacticalTableRowView.drawSeparator: comment claimed a "skip last
   row" behavior that wasn't actually implemented. Updated the
   comment to reflect reality (the host NSTableView controls whether
   drawSeparator is called for the last row via gridStyleMask /
   intercellSpacing).

#4 PacketFilterPresets: add `populateMenu(_:target:action:)` overload
   that appends preset items onto an existing menu. buildMenu now
   delegates to it. PacketAnalysisWindowController now calls
   populateMenu directly on the popup's existing menu, preserving
   the title item without the allocate-then-copy-every-item dance.

#5 VMConnectionOverlayView.draw: documented the latent horizontal-
   scroller caveat. `bounds.maxX` tracks content width (overlay is a
   table subview); when the table has no horizontal scroller (today)
   visible == content so this is fine. Comment now points future
   maintainers at `clipView.visibleRect.maxX` if/when that changes.

#6 AppDelegate.handleFocusVMConsole: switched from Swift string
   interpolation inside NSLog's format string to printf-style %@
   formatting matching the rest of the file (and avoiding format-
   string concerns).

#7 VMManager.networkPeers: added a perf comment documenting the
   O(routers × n) cost in refreshConnectionOverlay's loop and the
   `[routerVMId: [guests]]` index hint if the library ever grows
   into the hundreds of VMs.

#8 AppColorsTests.testCyanAliasesPointToODGreen: replaced NSColor
   instance equality with a component-by-component comparison via
   a new `assertSameColor` helper. Test now still passes if a
   future refactor expresses the alias as two literal NSColors with
   matching components — what matters is the *visible* contract,
   not Swift-level reference equality. Tolerance: 0.001 per channel.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant