diff --git a/Packages/OsaurusCore/Managers/Model/ModelManager.swift b/Packages/OsaurusCore/Managers/Model/ModelManager.swift index c87d515f6..dc6595695 100644 --- a/Packages/OsaurusCore/Managers/Model/ModelManager.swift +++ b/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 diff --git a/Packages/OsaurusCore/Tests/Work/WorkFolderToolsResolvePathTests.swift b/Packages/OsaurusCore/Tests/Work/WorkFolderToolsResolvePathTests.swift new file mode 100644 index 000000000..d0b5c1027 --- /dev/null +++ b/Packages/OsaurusCore/Tests/Work/WorkFolderToolsResolvePathTests.swift @@ -0,0 +1,117 @@ +// +// WorkFolderToolsResolvePathTests.swift +// osaurusTests +// +// Containment tests for `WorkFolderToolHelpers.resolvePath`. These exercise +// both the lexical (`..` rejection) and symlink-aware (`/Volumes/escape`) +// layers of path containment, plus the macOS-typical case where the work +// root itself is reached via a symlink and must not fail spuriously. +// + +import Foundation +import Testing + +@testable import OsaurusCore + +private struct TempDirectory { + let url: URL + + init(name: String = "osaurus-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 WorkFolderToolsResolvePathTests { + + @Test func acceptsRelativePathInsideRoot() throws { + let root = try TempDirectory() + defer { root.cleanup() } + + let resolved = try WorkFolderToolHelpers.resolvePath("src/lib.swift", rootPath: root.url) + #expect(resolved.path.hasPrefix(root.url.standardized.path)) + } + + @Test func acceptsLeadingSlashAsRelative() throws { + let root = try TempDirectory() + defer { root.cleanup() } + + let resolved = try WorkFolderToolHelpers.resolvePath("/src/lib.swift", rootPath: root.url) + #expect(resolved.path.hasPrefix(root.url.standardized.path)) + } + + @Test func rejectsDotDotEscape() throws { + let root = try TempDirectory() + defer { root.cleanup() } + + var threw = false + do { + _ = try WorkFolderToolHelpers.resolvePath("../etc/passwd", rootPath: root.url) + } catch WorkFolderToolError.pathOutsideRoot { + threw = true + } catch { + // Any other error is also OK as long as we reject it. + threw = true + } + #expect(threw) + } + + @Test func rejectsSymlinkEscape() throws { + let root = try TempDirectory() + defer { root.cleanup() } + + let outside = try TempDirectory(name: "osaurus-outside-\(UUID().uuidString)") + defer { outside.cleanup() } + + // Plant a symlink inside the work root that points outside it. An + // agent that emits `escape/secret` would lexically pass the + // `.standardized` prefix check but actually read from `outside/...`. + let symlinkInsideRoot = root.url.appendingPathComponent("escape") + try FileManager.default.createSymbolicLink( + at: symlinkInsideRoot, withDestinationURL: outside.url) + + var threw = false + do { + _ = try WorkFolderToolHelpers.resolvePath("escape/secret", rootPath: root.url) + } catch WorkFolderToolError.pathOutsideRoot { + threw = true + } catch { + threw = true + } + #expect(threw, "symlink that exits the work root must be rejected") + } + + @Test func acceptsRootReachedViaSymlink() throws { + // macOS-typical pattern: the work-root URL the caller hands us is + // itself reached through a symlink (e.g. /var -> /private/var on + // macOS). Containment must compare apples-to-apples after symlink + // resolution; otherwise legitimate projects fail. + let realRoot = try TempDirectory() + defer { realRoot.cleanup() } + + let symlinkParent = try TempDirectory(name: "osaurus-symlink-\(UUID().uuidString)") + defer { symlinkParent.cleanup() } + + let symlinkRoot = symlinkParent.url.appendingPathComponent("root") + try FileManager.default.createSymbolicLink( + at: symlinkRoot, withDestinationURL: realRoot.url) + + // Create an actual file under the real path to make the test concrete. + let nestedDir = realRoot.url.appendingPathComponent("src") + try FileManager.default.createDirectory(at: nestedDir, withIntermediateDirectories: true) + let nestedFile = nestedDir.appendingPathComponent("lib.swift") + FileManager.default.createFile(atPath: nestedFile.path, contents: Data()) + + let resolved = try WorkFolderToolHelpers.resolvePath("src/lib.swift", rootPath: symlinkRoot) + // Either the symlinked or symlink-resolved path is acceptable as the + // returned URL; the important property is that the call succeeded. + let symPath = symlinkRoot.appendingPathComponent("src/lib.swift").path + let realPath = nestedFile.path + #expect(resolved.path == symPath || resolved.path == realPath) + } +} diff --git a/Packages/OsaurusCore/Work/WorkFolderTools.swift b/Packages/OsaurusCore/Work/WorkFolderTools.swift index 224cc933c..57f729a31 100644 --- a/Packages/OsaurusCore/Work/WorkFolderTools.swift +++ b/Packages/OsaurusCore/Work/WorkFolderTools.swift @@ -32,7 +32,37 @@ enum WorkFolderToolError: LocalizedError { /// Shared utilities for folder tools enum WorkFolderToolHelpers { - /// Resolve and validate a relative path, ensuring it's within rootPath + /// Resolve and validate a relative path, ensuring it's within rootPath. + /// + /// The check runs twice: + /// + /// 1. Lexically (after `.standardized`): collapse `..` segments and + /// confirm the result still sits under `rootPath`. This catches the + /// overwhelmingly common case of an agent emitting `"../etc/passwd"` + /// without touching the filesystem. + /// 2. Symlink-aware: walk the relative components one at a time on top + /// of the symlink-resolved root, resolving any symlink encountered + /// along the way, and re-check containment. Without this second + /// pass, an attacker (or a careless project layout) could plant a + /// symlink inside the work root that points outside it; the agent + /// would then read or write through that link believing it was + /// contained. + /// + /// We don't just call `URL.resolvingSymlinksInPath()` on the full + /// candidate path because macOS uses strict-realpath semantics: when + /// any path component doesn't exist yet (e.g. `/root/escape/newfile` + /// where `newfile` hasn't been created), it returns the input + /// **unchanged** rather than resolving the components that DO exist. + /// That left `escape`'s symlink target undetected and let the test + /// for "symlink escape rejected" pass silently in CI. Walking + /// components manually and resolving each existing prefix gets us + /// realpath -m semantics. + /// + /// Containment is compared by `pathComponents` arrays with + /// `Array.starts(with:)` so macOS's `/var` ↔ `/private/var` + /// canonicalization can't produce a one-sided string mismatch, and so + /// a sibling whose name shares a prefix (e.g. `/work/foo-baz` vs + /// `/work/foo`) can't slip through. static func resolvePath(_ relativePath: String, rootPath: URL) throws -> URL { let cleanPath = relativePath.hasPrefix("/") ? String(relativePath.dropFirst()) : relativePath let resolvedURL = rootPath.appendingPathComponent(cleanPath).standardized @@ -41,9 +71,49 @@ enum WorkFolderToolHelpers { guard resolvedURL.path.hasPrefix(rootPathString) else { throw WorkFolderToolError.pathOutsideRoot(relativePath) } + + // Symlink-aware containment. + let canonicalRoot = rootPath.resolvingSymlinksInPath().standardized + let canonicalCandidate = Self.resolveExistingComponents( + relativeTo: canonicalRoot, + relativePath: cleanPath + ) + guard canonicalCandidate.pathComponents.starts(with: canonicalRoot.pathComponents) else { + throw WorkFolderToolError.pathOutsideRoot(relativePath) + } return resolvedURL } + /// Walk `relativePath` one component at a time on top of `rootPath`, + /// resolving symlinks for every component that currently exists on + /// disk and appending the remainder lexically once we run off the end + /// of the existing tree. This is the moral equivalent of `realpath -m` + /// on Linux — exactly what we need for path containment when the + /// caller is asking about a file that may not have been created yet. + private static func resolveExistingComponents( + relativeTo rootPath: URL, + relativePath: String + ) -> URL { + let fm = FileManager.default + let parts = relativePath.split(separator: "/").map(String.init) + var current = rootPath + var stillResolving = true + + for part in parts { + // Defensive: `/foo//bar` would yield an empty part, which + // `appendingPathComponent` rejects. Skip them. + if part.isEmpty { continue } + let next = current.appendingPathComponent(part) + if stillResolving && fm.fileExists(atPath: next.path) { + current = next.resolvingSymlinksInPath() + } else { + stillResolving = false + current = current.appendingPathComponent(part) + } + } + return current.standardized + } + /// Parse JSON arguments to dictionary static func parseArguments(_ json: String) throws -> [String: Any] { guard let data = json.data(using: .utf8),