Security: validate extracted archive trees against ZIP-slip / symlink-escape

Packages/OsaurusCLI/Sources/OsaurusCLICore/Commands/Bundle/MCPBundleManager.swift

@@ -7,6 +7,7 @@
 
 import Foundation
 import MCP
+import OsaurusRepository
 
 class MCPBundleManager {
     struct BundleInfo {
@@ -115,6 +116,17 @@ class MCPBundleManager {
         // Use unzip CLI
         try unzipWithCLI(bundlePath, to: extractPath)
 
+        // Defense in depth: reject ZIP-slip and symlink escape entries
+        // before reading the bundle manifest. Bundles are loaded from
+        // local paths the user picked, so they're trusted-ish, but
+        // validating costs almost nothing and shrinks the trust surface.
+        do {
+            try ArchiveSafety.validate(extractedRoot: URL(fileURLWithPath: extractPath))
+        } catch {
+            try? FileManager.default.removeItem(atPath: extractPath)
+            throw BundleLoadError.extractionFailed("Unsafe bundle layout: \(error)")
+        }
+
         let manifestPath = "\(extractPath)/manifest.json"
         guard FileManager.default.fileExists(atPath: manifestPath) else {
             try? FileManager.default.removeItem(atPath: extractPath)

Packages/OsaurusCLI/Sources/OsaurusCLICore/Commands/Tools/ToolsInstall.swift

@@ -86,6 +86,7 @@ public struct ToolsInstall {
                 }
                 try data.write(to: zipFile)
                 try unzip(zipURL: zipFile, to: tmpDir)
+                try ArchiveSafety.validate(extractedRoot: tmpDir)
             } catch {
                 fputs("Download/Unzip error: \(error)\n", stderr)
                 exit(EXIT_FAILURE)
@@ -109,6 +110,7 @@ public struct ToolsInstall {
                 } else if pathURL.pathExtension.lowercased() == "zip" {
                     do {
                         try unzip(zipURL: pathURL, to: tmpDir)
+                        try ArchiveSafety.validate(extractedRoot: tmpDir)
                     } catch {
                         fputs("Unzip error: \(error)\n", stderr)
                         exit(EXIT_FAILURE)

Packages/OsaurusCore/Managers/Model/ModelManager.swift

@@ -188,7 +188,27 @@ final class ModelManager: NSObject, ObservableObject {
 
         // Pull the OsaurusAI HF org listing once on launch so newly published
         // models surface in the Recommended tab without requiring a code push.
-        Task { [weak self] in await self?.loadOsaurusAIOrgModels() }
+        //
+        // The unit-test runner constructs `ModelManager()` repeatedly to drive
+        // `applyOsaurusOrgFetch` directly. If the launch-time HF fetch races
+        // with those test calls, whichever finishes last wins and the merge
+        // result is non-deterministic — that's the regression class behind
+        // `ModelManagerSuggestedTests/applyOsaurusOrgFetch_*` flaking in CI.
+        // Skip the background fetch under XCTest; production launches still
+        // get it because `XCTestConfigurationFilePath` is only set inside
+        // a test host.
+        if !Self.isRunningInTestEnvironment {
+            Task { [weak self] in await self?.loadOsaurusAIOrgModels() }
+        }
+    }
+
+    /// True when the current process was launched by xctest. Used to gate
+    /// network-touching launch-time side effects so tests can drive the
+    /// affected code paths deterministically.
+    nonisolated private static var isRunningInTestEnvironment: Bool {
+        ProcessInfo.processInfo.environment["XCTestConfigurationFilePath"] != nil
+            || ProcessInfo.processInfo.environment["XCTestBundlePath"] != nil
+            || ProcessInfo.processInfo.environment["XCTestSessionIdentifier"] != nil
     }
 
     // MARK: - Public Methods

Packages/OsaurusCore/Managers/SkillManager.swift

@@ -7,6 +7,7 @@
 
 import Foundation
 import Observation
+import OsaurusRepository
 import SwiftUI
 
 public enum SkillFileError: Error, LocalizedError {
@@ -311,6 +312,15 @@ public final class SkillManager {
         try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true)
         try await FileManager.default.unzipItem(at: zipURL, to: tempDir)
 
+        // Defense in depth: skill archives are user-provided (drag-and-drop)
+        // and unsigned. Reject ZIP-slip and symlink-escape entries before
+        // any skill metadata is read or copied into the skills directory.
+        do {
+            try ArchiveSafety.validate(extractedRoot: tempDir)
+        } catch {
+            throw SkillFileError.invalidSkillArchive
+        }
+
         guard let skillMdURL = findSkillMd(in: tempDir) else {
             throw SkillFileError.invalidSkillArchive
         }

Packages/OsaurusCore/Tests/Plugin/ArchiveSafetyTests.swift

@@ -0,0 +1,100 @@
+//
+//  ArchiveSafetyTests.swift
+//  osaurusTests
+//
+//  Tests for the post-extraction ZIP-slip / symlink-escape validator in
+//  `OsaurusRepository.ArchiveSafety`. The validator runs after the extractor
+//  (currently /usr/bin/unzip) has written files to disk and is the last line
+//  of defense before the caller reads anything out of the extracted tree.
+//
+
+import Foundation
+import OsaurusRepository
+import Testing
+
+@testable import OsaurusCore
+
+private struct TempDirectory {
+    let url: URL
+
+    init(name: String = "osaurus-archive-tests-\(UUID().uuidString)") throws {
+        let dir = FileManager.default.temporaryDirectory.appendingPathComponent(name, isDirectory: true)
+        try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true)
+        self.url = dir
+    }
+
+    func cleanup() {
+        try? FileManager.default.removeItem(at: url)
+    }
+}
+
+struct ArchiveSafetyTests {
+
+    @Test func acceptsCleanTree() throws {
+        let root = try TempDirectory()
+        defer { root.cleanup() }
+
+        let nestedDir = root.url.appendingPathComponent("src")
+        try FileManager.default.createDirectory(at: nestedDir, withIntermediateDirectories: true)
+        FileManager.default.createFile(
+            atPath: nestedDir.appendingPathComponent("lib.swift").path,
+            contents: Data("hi".utf8)
+        )
+        FileManager.default.createFile(
+            atPath: root.url.appendingPathComponent("README.md").path,
+            contents: Data("readme".utf8)
+        )
+
+        try ArchiveSafety.validate(extractedRoot: root.url)
+    }
+
+    @Test func rejectsSymlinkEscape() throws {
+        let root = try TempDirectory()
+        defer { root.cleanup() }
+        let outside = try TempDirectory(name: "osaurus-archive-outside-\(UUID().uuidString)")
+        defer { outside.cleanup() }
+
+        // Simulate a malicious zip entry that, when extracted, became a
+        // symbolic link inside the destination pointing OUT of it. macOS
+        // Info-ZIP's own protections don't catch this case.
+        let link = root.url.appendingPathComponent("escape")
+        try FileManager.default.createSymbolicLink(at: link, withDestinationURL: outside.url)
+
+        var threw = false
+        do {
+            try ArchiveSafety.validate(extractedRoot: root.url)
+        } catch ArchiveSafetyError.escapingSymlink {
+            threw = true
+        } catch {
+            threw = true
+        }
+        #expect(threw, "symlink to outside directory must be rejected")
+    }
+
+    @Test func acceptsInternalSymlink() throws {
+        let root = try TempDirectory()
+        defer { root.cleanup() }
+
+        let target = root.url.appendingPathComponent("real-file")
+        FileManager.default.createFile(atPath: target.path, contents: Data())
+
+        let link = root.url.appendingPathComponent("alias")
+        try FileManager.default.createSymbolicLink(at: link, withDestinationURL: target)
+
+        try ArchiveSafety.validate(extractedRoot: root.url)
+    }
+
+    @Test func rejectsExtractionRootMissing() {
+        let missing = FileManager.default.temporaryDirectory
+            .appendingPathComponent("osaurus-archive-missing-\(UUID().uuidString)")
+        var threw = false
+        do {
+            try ArchiveSafety.validate(extractedRoot: missing)
+        } catch ArchiveSafetyError.extractionRootMissing {
+            threw = true
+        } catch {
+            threw = true
+        }
+        #expect(threw)
+    }
+}

Packages/OsaurusRepository/ArchiveSafety.swift

@@ -0,0 +1,121 @@
+//
+//  ArchiveSafety.swift
+//  OsaurusRepository
+//
+//  Post-extraction validation that catches ZIP-slip and symlink-escape
+//  attempts. After extracting an archive into a destination directory,
+//  call `ArchiveSafety.validate(extractedRoot:)` and abort the install
+//  if it throws. Validation is symlink-aware: every regular file, every
+//  directory, and every symlink target is required to resolve to a path
+//  inside the destination root.
+//
+//  Why post-extraction:
+//    * Streaming-time validation would require a ZIP library; we currently
+//      shell out to /usr/bin/unzip and the same logic is duplicated across
+//      callers (PluginInstallManager, SkillManager, CLI ToolsInstall).
+//    * The destination root is a freshly-created temp directory the caller
+//      owns. Until validate() succeeds, the caller must not move or read
+//      the extracted contents.
+//    * Symlinks introduced by the archive (or by a sloppy build) are the
+//      attack vector left over once the unzip tool itself starts rejecting
+//      absolute / ".." entries. macOS Info-ZIP already rejects absolute
+//      paths and many ".." escapes; symlinks pointing outside the tree are
+//      what we still have to police.
+//
+
+import Foundation
+
+public enum ArchiveSafetyError: Error, CustomStringConvertible {
+    case escapingPath(String)
+    case escapingSymlink(linkPath: String, target: String)
+    case extractionRootMissing(String)
+
+    public var description: String {
+        switch self {
+        case .escapingPath(let p):
+            return "Archive entry resolves outside the extraction root: \(p)"
+        case .escapingSymlink(let link, let target):
+            return "Archive symlink \(link) points outside the extraction root: \(target)"
+        case .extractionRootMissing(let p):
+            return "Extraction root does not exist or is not a directory: \(p)"
+        }
+    }
+}
+
+public enum ArchiveSafety {
+
+    /// Walk `extractedRoot` and verify every entry is contained — both
+    /// lexically (no `..` escapes) and after symlink resolution.
+    ///
+    /// Throws `ArchiveSafetyError` on the first escape encountered.
+    /// Callers should treat the entire extracted tree as untrusted and
+    /// remove it on failure.
+    ///
+    /// Containment is compared by *path components* (after both sides go
+    /// through `resolvingSymlinksInPath().standardized`) rather than by
+    /// string prefix. On macOS the TemporaryDirectory and the URLs the
+    /// `FileManager.enumerator` returns are canonicalized inconsistently
+    /// (`/var` vs `/private/var` depending on the API), and a string
+    /// `hasPrefix` check produced false positives whenever one side was
+    /// resolved and the other wasn't. `Array.starts(with:)` on
+    /// `pathComponents` avoids that mismatch and is also immune to the
+    /// "shared name prefix" foot-gun (e.g. `/work/foo-baz` vs
+    /// `/work/foo`).
+    public static func validate(extractedRoot: URL) throws {
+        let fm = FileManager.default
+        var isDir: ObjCBool = false
+        guard fm.fileExists(atPath: extractedRoot.path, isDirectory: &isDir), isDir.boolValue else {
+            throw ArchiveSafetyError.extractionRootMissing(extractedRoot.path)
+        }
+
+        // Canonicalize the root once. We compare candidate paths against
+        // BOTH the symlink-resolved form (catches a symlink whose target
+        // exits the root) AND the lexically-standardized form (catches a
+        // candidate whose own enumerator URL was returned with a different
+        // /var ↔ /private/var canonicalization than the root).
+        let canonicalRoot = extractedRoot.resolvingSymlinksInPath().standardized
+        let lexicalRoot = extractedRoot.standardized
+        let canonicalRootComponents = canonicalRoot.pathComponents
+        let lexicalRootComponents = lexicalRoot.pathComponents
+
+        guard
+            let enumerator = fm.enumerator(
+                at: extractedRoot,
+                includingPropertiesForKeys: [.isSymbolicLinkKey],
+                options: []
+            )
+        else {
+            return
+        }
+
+        for case let item as URL in enumerator {
+            // 1. Lexical containment — catches '..' / absolute-path entries
+            //    that slipped past unzip. Compared against the lexically-
+            //    standardized root.
+            let lexicalCandidate = item.standardized.pathComponents
+            let passesLexical = lexicalCandidate.starts(with: lexicalRootComponents)
+
+            // 2. Symlink-aware containment — catches a symlink whose target
+            //    exits the root. Compared against the symlink-resolved root.
+            let canonicalCandidate = item.resolvingSymlinksInPath().standardized.pathComponents
+            let passesCanonical = canonicalCandidate.starts(with: canonicalRootComponents)
+
+            // Either canonicalization being inside is enough; macOS's
+            // /var ↔ /private/var inconsistency means one form or the
+            // other may match while the other doesn't, and both forms
+            // refer to the same real file.
+            guard passesLexical || passesCanonical else {
+                let isSymlink =
+                    (try? item.resourceValues(forKeys: [.isSymbolicLinkKey]))?
+                    .isSymbolicLink ?? false
+                if isSymlink {
+                    let target =
+                        (try? fm.destinationOfSymbolicLink(atPath: item.path))
+                        ?? item.resolvingSymlinksInPath().path
+                    throw ArchiveSafetyError.escapingSymlink(linkPath: item.path, target: target)
+                }
+                throw ArchiveSafetyError.escapingPath(item.path)
+            }
+        }
+    }
+}

Packages/OsaurusRepository/PluginInstallManager.swift

@@ -128,6 +128,19 @@ public final class PluginInstallManager: @unchecked Sendable {
         defer { try? FileManager.default.removeItem(at: tmpDir) }
         try unzip(zipURL: tmpZip, to: tmpDir)
 
+        // Defense in depth against ZIP-slip and symlink escape. macOS
+        // Info-ZIP already rejects absolute and ".."-laden entries by
+        // default, but symlinks inside the archive whose targets exit the
+        // extraction root still extract. The bundle is signature-verified
+        // above, so the threat surface here is mostly "publisher key
+        // compromise" — but the cost of validating is trivial relative to
+        // the cost of recovering from a successful escape.
+        do {
+            try ArchiveSafety.validate(extractedRoot: tmpDir)
+        } catch {
+            throw PluginInstallError.layoutInvalid("Archive contains escaping entry: \(error)")
+        }
+
         guard let dylibURL = findFirstDylib(in: tmpDir) else {
             throw PluginInstallError.layoutInvalid("No .dylib found in archive")
         }