harden(scaffold): whole-branch-review fixes — EgressAllowlist hostname validation, detach-ordering + timeout test hygiene, deps-persistence/seed comments

SEC-1: validate each EgressAllowlist entry against ^(?i)\.?[a-z0-9][a-z0-9.-]*$
in Assert-TierProfileValid (ProfileLoader.ps1) before it reaches the builder's
Squid dstdomain ACL (SeedBuilder substitutes the allowlist with no escaping; a
newline-bearing entry could inject http_access allow all). Runs on bare tiers
AND the merged builder. All shipped profiles (tier0/tier1/ralph/firefox/builder)
still load. New reject + accept tests; SeedBuilder xref comment.

TR-2: the detach-before-hash test used IndexOf ordering only, which is vacuously
true when an op is absent (-1 -BeLessThan positive). Added Should -Contain guards
for RemoveHardDiskDrive + GetVhdxImageHash before the ordering assertion.

TR-3: the two builder timeout tests leaked a "did not power off" WARNING into
Pester output. Capture it (-WarningVariable) and assert it (a real timeout-path
check), and silence the host leak (-WarningAction SilentlyContinue).

SPEC-1/ci-1: BuilderVM.ps1 comments claimed deps.vhdx "is left on disk" — but the
default DepsDiskPath == OutputDiskPath IS in CreatedDisks and IS deleted at
teardown. Rewrote the .DESCRIPTION + finally-block comments: persistence is
LIVE-only pending the Phase-6 Copy-Item; the whole-image hash is the durable
artifact today.

2.2 docstring: SeedBuilder header "Two shapes" -> "Three" (DISK-OFFLINE,
DISK-BUILDER [Disk+SquidSniProxy], SERIAL).

SEC-2 (comment only): the builder seed redirects TCP 80/443 to Squid but does NOT
yet enforce BlockProtocols (DNS/53, QUIC/UDP-443, DoH/DoT) — noted as Phase-6;
no enforcement wired.

ci-2: SeedIso is genuinely unused in the disk-mode/builder path (New-WorkloadSeedDisk
builds the seed onto a CIDATA data disk; SeedIso is consumed only by the serial/DVD
path). Added a clarifying NOTE in builder.psd1; kept the key (the loader carries it,
mirrors firefox.psd1).

Author: allenfbyrd

profiles/builder.psd1

@@ -52,7 +52,12 @@
     # deps + a per-file SHA-256 manifest under /mnt/out (-> the OUTPUT disk -> deps.vhdx).
     Entrypoint = 'python3 /mnt/in/fetch_deps.py --spec /mnt/in/deps-spec.json --out /mnt/out'
 
-    # The cloud-init NoCloud CIDATA seed (2.2 builds the builder variant with the Squid proxy +
-    # dep-fetch runner). Attached read-only at provision; ejected by the seal. Not secret-shaped.
+    # The cloud-init NoCloud CIDATA seed path. ci-2 NOTE: this is UNUSED in the disk-mode/builder path —
+    # the builder is WorkloadMode='Disk', and New-WorkloadSeedDisk (Workload.ps1) builds the seed CONTENT
+    # in-line onto a recorded CIDATA *data disk* ('<name>-cidata.vhdx' under the storage root, RC6) that
+    # survives the seal; it never reads this SeedIso file. SeedIso is consumed ONLY by the serial/DVD path
+    # (New-CidataSeed -Destination default + Add-SandboxSeed). Kept (not dropped) because the loader still
+    # carries the key (mirrors firefox.psd1, which also declares it for disk-mode) and it documents the
+    # canonical seed path; it is harmless in disk mode. Not secret-shaped.
     SeedIso = 'C:\sandbox\assets\cidata-seed.iso'
 }

scripts/lib/BuilderVM.ps1

@@ -22,15 +22,20 @@
                   locked; the fake THROWS on GetVhdxImageHash while still attached).
       HASH        GetVhdxImageHash — whole-file SHA-256 in USER-SPACE (NEVER Mount-VHD).
       EMIT        Record DepsDiskPath + WholeImageHash + Status=Success.
-      TEARDOWN    Remove-Sandbox in a finally — leaves deps.vhdx (NOT in CreatedDisks; M2).
+      TEARDOWN    Remove-Sandbox in a finally. NOTE: in the DEFAULT case $DepsDiskPath == OutputDiskPath,
+                  which New-WorkloadDisks PUT in CreatedDisks, so Remove-Sandbox -DeleteDisks DELETES it.
+                  deps.vhdx persistence is LIVE-only, pending the Phase-6 Copy-Item to a stable path that
+                  is NOT in CreatedDisks (the commented-out Copy-Item below the hash). Until then the
+                  whole-image hash is the durable artifact; the deps .vhdx itself does not survive teardown.
 
     DESIGN DECISIONS (resolved — do not redesign):
       * No host-mount: the integrity artifact is the whole-image hash (GetVhdxImageHash, raw).
       * Success = clean power-off: -not $wait.TimedOut -> Status='Success'; timeout -> 'Failed'.
         The exitcode/per-file-manifest outbox read DEFERS to Phase 4.
-      * deps.vhdx persists (M2): teardown LEAVES deps.vhdx (not in CreatedDisks). The real file
-        copy to a stable path is LIVE-only (Phase 6); in the mock set DepsDiskPath to the
-        OUTPUT path and record the hash.
+      * deps.vhdx persistence is LIVE-only (Phase 6): in the DEFAULT case DepsDiskPath == OutputDiskPath,
+        which IS in CreatedDisks, so teardown's Remove-Sandbox -DeleteDisks DELETES it. The Phase-6
+        Copy-Item to a stable path NOT in CreatedDisks is what will make deps.vhdx survive; until then the
+        mock just sets DepsDiskPath to the OUTPUT path and records the hash (the durable artifact).
 
     Dot-source this file AFTER Invoke-Voidseal.ps1 (which dot-sources all engine libs:
     HyperVBackend.ps1, ProfileLoader.ps1, Provisioner.ps1, Sealer.ps1, Runner.ps1,
@@ -222,10 +227,12 @@ function Invoke-BuilderVM {
     }
     finally {
         # --- DESTROYED: teardown ALWAYS runs (no orphaned VM/disk/switch) ---
-        # LEAVE deps.vhdx (M2): the deps disk is the builder's persistent output artifact, NOT
-        # in CreatedDisks (we never added it). Remove-Sandbox -DeleteDisks only targets
-        # CreatedDisks (the system disk + INPUT + OUTPUT + seed disk), so deps.vhdx is left
-        # on disk whether we're using the stable $DepsDiskPath or the working OutputDiskPath.
+        # deps.vhdx persistence is LIVE-only (Phase 6). In the DEFAULT case $DepsDiskPath ==
+        # OutputDiskPath, which New-WorkloadDisks PUT in CreatedDisks (the system disk + INPUT +
+        # OUTPUT + seed disk), so Remove-Sandbox -DeleteDisks DELETES it here — deps.vhdx does NOT
+        # survive teardown today. The Phase-6 Copy-Item (commented out above the EMIT) copies the
+        # OUTPUT .vhdx to a stable path NOT in CreatedDisks; only THEN does the deps artifact persist.
+        # Until then the whole-image hash is the durable record, not the .vhdx file.
         try {
             if ($null -ne $descriptor) {
                 Remove-Sandbox -Name $Name -DeleteDisks -Descriptor $descriptor -Backend $Backend

scripts/lib/ProfileLoader.ps1

@@ -282,6 +282,24 @@ function Assert-TierProfileValid {
     # EgressAllowlist must be an array (already normalized by caller, but re-check shape).
     $allowlist = @($Profile['EgressAllowlist'])
 
+    # SEC-1: hostname-charset validation. The builder substitutes ($allowlist -join ' ') straight into
+    # a Squid `dstdomain` ACL (SeedBuilder.ps1) with NO escaping; an entry bearing a newline (or quote/
+    # whitespace) could inject a directive such as `http_access allow all` and defeat the egress ACL.
+    # Validate EACH entry against a strict hostname charset (optional leading dot for a domain-suffix
+    # match; the charset excludes whitespace/newline/quote/slash), failing closed and naming the bad
+    # entry. Runs on bare tiers AND the merged builder, so the allowlist is gatekept before it ever
+    # reaches the Squid seed. (Tier>=2 enforces an EMPTY allowlist below, so this is a no-op there.)
+    $hostnameRe = '^(?i)\.?[a-z0-9][a-z0-9.-]*$'
+    foreach ($entry in $allowlist) {
+        $e = [string]$entry
+        if ($e -notmatch $hostnameRe) {
+            throw ("Schema validation: '$Context' field 'EgressAllowlist' has an invalid entry '$e' " +
+                   "(must match $hostnameRe — a hostname/domain-suffix, optional leading dot, no whitespace/" +
+                   "newline/quote/slash). An unvalidated entry could inject directives into the builder's " +
+                   "Squid dstdomain ACL. Failing closed.")
+        }
+    }
+
     # HostChannels must be a hashtable.
     if (-not ($Profile['HostChannels'] -is [System.Collections.IDictionary])) {
         throw "Schema validation: '$Context' field 'HostChannels' must be a hashtable."

scripts/lib/SeedBuilder.ps1

@@ -4,13 +4,17 @@
 
 .DESCRIPTION
     Builds the first-boot **NoCloud `CIDATA` seed ISO** a sandbox guest consumes on its FIRST boot.
-    Two shapes, selected by the profile's `WorkloadMode`:
+    Three shapes, selected by the profile's `WorkloadMode` (+ `EgressMode` for the builder variant):
 
-      * DISK  — the disk-passing workload runner (guest-images/debian-12-cloud.md §2a). A systemd
-        oneshot mounts the host-pre-formatted exFAT data disks by LABEL (INPUT ro -> /mnt/in,
+      * DISK-OFFLINE — the disk-passing workload runner (guest-images/debian-12-cloud.md §2a). A
+        systemd oneshot mounts the host-pre-formatted exFAT data disks by LABEL (INPUT ro -> /mnt/in,
         OUTPUT rw -> /mnt/out), runs the profile's `Entrypoint` as the non-root `sandbox` user,
         writes `result.html` + the `result.exitcode` sentinel onto OUTPUT, flushes, and self-powers
         off. The seed builder substitutes the profile's `Entrypoint` for the `__ENTRYPOINT__` token.
+      * DISK-BUILDER (`WorkloadMode='Disk'` + `EgressMode='SquidSniProxy'`) — the Tier-1 net-restricted
+        builder variant: same disk-passing runner PLUS a transparent Squid SNI domain-ACL proxy that
+        gatekeeps the guest's 80/443 egress to the `EgressAllowlist` only (substituted into the Squid
+        `dstdomain` ACL). Fetches deps over Squid into /mnt/out, writes a manifest, self-powers-off.
       * SERIAL (default) — the §2 baseline: serial-getty AUTOLOGIN on ttyS0 (the Runner's command
         seam, which does NOT authenticate), a non-root run-user, and bubblewrap.
 
@@ -210,6 +214,10 @@ write_files:
       # --- Squid transparent proxy setup ---
       systemctl restart squid 2>/dev/null || true
       # Redirect outbound HTTP/HTTPS through Squid (transparent intercept)
+      # SEC-2 (HONESTY — deferred to Phase 6): this only REDIRECTS TCP 80/443 to Squid. It does NOT yet
+      # enforce tier1's BlockProtocols (DNS/53, QUIC/UDP-443, DoH/DoT) — only 80/443 are gatekept by the
+      # domain ACL; DNS and any non-80/443 egress are NOT default-dropped. Full default-DROP egress
+      # enforcement (the BlockProtocols set) is Phase-6 work and is intentionally NOT wired here.
       iptables -t nat -A OUTPUT -p tcp --dport 80  -m owner ! --uid-owner proxy -j REDIRECT --to-port 3129
       iptables -t nat -A OUTPUT -p tcp --dport 443 -m owner ! --uid-owner proxy -j REDIRECT --to-port 3130
 
@@ -344,6 +352,9 @@ function New-CidataUserData {
         if ($null -eq $allowlist -or @($allowlist).Count -eq 0) {
             throw "New-CidataUserData: a builder Disk+SquidSniProxy profile must have a non-empty EgressAllowlist (allowlist is empty — fail-closed)."
         }
+        # SEC-1: each allowlist entry's charset (no whitespace/newline/quote) is validated at LOAD time
+        # in Assert-TierProfileValid (ProfileLoader.ps1) before it ever reaches here, so a newline-bearing
+        # entry cannot inject a directive (e.g. `http_access allow all`) into the dstdomain ACL below.
         $aclLine = ($allowlist -join ' ')
         $ud = $script:CidataBuilderRunnerTemplate.Replace('__ENTRYPOINT__', $entrypoint).Replace('__SQUID_ALLOWLIST_ACL__', $aclLine)
         return (ConvertTo-LfText -Text $ud)

tests/BuilderVM.Tests.ps1

@@ -49,14 +49,22 @@ Describe 'Invoke-BuilderVM — Tier-1 builder orchestration (Phase 2.4)' {
         $r = Invoke-BuilderVM -Profile "$script:SkillRoot/profiles/builder.psd1" -Name 'bld2' -Backend $fake -DepsDiskPath (Join-Path $TestDrive 'd2.vhdx')
         $r.Status | Should -Be 'Success'   # GetVhdxImageHash THROWS if still attached -> Success proves detach-before-hash
         $ops = @($fake.FakeCallLog | Where-Object { $_.Op -in @('RemoveHardDiskDrive','GetVhdxImageHash') } | ForEach-Object { $_.Op })
+        # TR-2: assert BOTH ops actually ran BEFORE the IndexOf ordering check — IndexOf returns -1 when an
+        # op is absent, and `-1 -BeLessThan <positive>` is vacuously TRUE, so the ordering assertion alone
+        # would pass even if the detach never happened (mirror InvokeVoidseal.Tests.ps1:352-363).
+        $ops | Should -Contain 'RemoveHardDiskDrive' -Because 'the OUTPUT/deps disk must be detached'
+        $ops | Should -Contain 'GetVhdxImageHash'    -Because 'the whole-image hash must run'
         ($ops.IndexOf('RemoveHardDiskDrive')) | Should -BeLessThan ($ops.IndexOf('GetVhdxImageHash'))
     }
 
     It 'a hung builder (SimulateNeverOff) times out -> Status=Failed, NO deps artifact, no hash' {
         $fake = New-FakeHyperVBackend -SimulateNeverOff -SimulateDepsImageBlob ([byte[]](1..8))
-        $r = Invoke-BuilderVM -Profile "$script:SkillRoot/profiles/builder.psd1" -Name 'bld3' -Backend $fake -WorkloadTimeoutSeconds 0 -DepsDiskPath (Join-Path $TestDrive 'd3.vhdx')
+        # TR-3: capture the lifecycle-abort WARNING (instead of letting it leak into Pester output) and
+        # assert it — turning the warning into a real behavioral check that the timeout path was hit.
+        $r = Invoke-BuilderVM -Profile "$script:SkillRoot/profiles/builder.psd1" -Name 'bld3' -Backend $fake -WorkloadTimeoutSeconds 0 -DepsDiskPath (Join-Path $TestDrive 'd3.vhdx') -WarningVariable wv -WarningAction SilentlyContinue
         $r.Status | Should -Be 'Failed'
         $r.WholeImageHash | Should -BeNullOrEmpty
+        (@($wv) -join "`n") | Should -Match 'did not power off' -Because 'the timeout path must emit the lifecycle-abort warning'
     }
 
     It 'a non-builder profile (EgressMode != SquidSniProxy) is refused' {
@@ -80,8 +88,10 @@ Describe 'Invoke-BuilderVM — teardown leaves deps.vhdx, removes other disks' {
 
     It 'the VM is removed after a timed-out builder run (no orphan)' {
         $fake = New-FakeHyperVBackend -SimulateNeverOff
-        $r = Invoke-BuilderVM -Profile "$script:SkillRoot/profiles/builder.psd1" -Name 'bld-timeout-td' -Backend $fake -WorkloadTimeoutSeconds 0 -DepsDiskPath (Join-Path $TestDrive 'deps-t2.vhdx')
+        # TR-3: capture + assert the lifecycle-abort warning (no leak into Pester output).
+        $r = Invoke-BuilderVM -Profile "$script:SkillRoot/profiles/builder.psd1" -Name 'bld-timeout-td' -Backend $fake -WorkloadTimeoutSeconds 0 -DepsDiskPath (Join-Path $TestDrive 'deps-t2.vhdx') -WarningVariable wv -WarningAction SilentlyContinue
         $r.Status | Should -Be 'Failed'
+        (@($wv) -join "`n") | Should -Match 'did not power off' -Because 'the timeout path must emit the lifecycle-abort warning'
         (& $fake.GetVM @{ Name = 'bld-timeout-td' }) | Should -BeNullOrEmpty -Because 'the builder VM is removed even on a timeout'
     }
 }
@@ -119,7 +129,7 @@ Describe 'Invoke-BuilderVM — RC7: SetAutomaticCheckpoints called before StartV
 
     It 'SetAutomaticCheckpoints(Enabled=$false) is applied before StartVM: the deps hash is non-empty (AutomaticCheckpoints=ON would hide the blob in a child .avhdx -> empty hash)' {
         # When AutomaticCheckpointsEnabled is still ON at StartVM, the fake routes the
-        # DepsImageBlob into OutboxChildLayer (the .avhdx child — inaccessible to GetVhdxImageHash,
+        # DepsImageBlob into DepsImageChildLayer (the .avhdx child — inaccessible to GetVhdxImageHash,
         # which reads the base). So GetVhdxImageHash would return SHA256 of empty bytes, NOT the
         # blob's hash. If RC7 is applied (Enabled=$false) before StartVM, the blob goes into
         # DepsImageRegion (the base layer) and the hash matches the blob. So a non-empty/correct

tests/ProfileLoader.Tests.ps1

@@ -626,6 +626,31 @@ Describe 'Builder profile + SquidSniProxy egress (Phase 2.1)' {
         Test-AllowlistCoversHost -Allowlist @('huggingface.co') -HostName 'cdn-lfs.huggingface.co' | Should -BeFalse
         Test-AllowlistCoversHost -Allowlist @('pypi.org') -HostName 'pypi.org' | Should -BeTrue
     }
+    It 'SEC-1: an EgressAllowlist entry bearing a newline (Squid ACL injection) is REFUSED fail-closed, naming the bad entry' {
+        # A newline-bearing entry could inject `http_access allow all` into the builder's Squid dstdomain
+        # ACL (SeedBuilder substitutes ($allowlist -join ' ') with no escaping). The load-time charset
+        # check must reject it before it ever reaches the seed. Direct Assert-TierProfileValid on a
+        # network-tier hashtable (a .psd1 cannot even carry a literal newline-in-a-string cleanly).
+        $bad = @{
+            Tier=1; Description='p'; Substrate='HyperV-Gen2'; Network='Internal'; EgressMode='NftablesAllowlist';
+            EgressAllowlist=@("pypi.org`nhttp_access allow all"); Credentials='None'; GuestImage='debian-12'; Memory=2GB; Cpu=2;
+            HostChannels=@{Clipboard=$false;Shares=$false;GuestServices=$false;EnhancedSession=$false};
+            Capture='HostReadResultDir'; Extraction='HostReadResultDir'; Lifecycle='CreateDestroy';
+            Controls=@(); ManagementChannel='Com1Serial'
+        }
+        { Assert-TierProfileValid -Profile $bad -Context 'sec1' } |
+            Should -Throw -ExpectedMessage '*EgressAllowlist*invalid entry*'
+    }
+    It 'SEC-1: a clean hostname AND a leading-dot domain-suffix entry are ACCEPTED (the regex passes real allowlists)' {
+        $ok = @{
+            Tier=1; Description='p'; Substrate='HyperV-Gen2'; Network='Internal'; EgressMode='NftablesAllowlist';
+            EgressAllowlist=@('cas-bridge.xethub.hf.co', '.hf.co', 'pypi.org'); Credentials='None'; GuestImage='debian-12'; Memory=2GB; Cpu=2;
+            HostChannels=@{Clipboard=$false;Shares=$false;GuestServices=$false;EnhancedSession=$false};
+            Capture='HostReadResultDir'; Extraction='HostReadResultDir'; Lifecycle='CreateDestroy';
+            Controls=@(); ManagementChannel='Com1Serial'
+        }
+        { Assert-TierProfileValid -Profile $ok -Context 'sec1ok' } | Should -Not -Throw
+    }
     It 'a workload overriding EgressMode to a NON-SquidSniProxy value is REFUSED' {
         $wl = Join-Path $TestDrive 'bad-egress.psd1'
         Set-Content -LiteralPath $wl -Encoding utf8 -Value @'