From 49a501653457898782e1ef4ee424a15a8c73d693 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 27 May 2026 04:20:24 +0000 Subject: [PATCH 1/4] Symlink-aware path containment in WorkFolderToolHelpers.resolvePath The existing containment check used only `.standardized`, which is a lexical operation that collapses '..' and '/./' but does not follow symbolic links. That made the agent's file tools vulnerable to symlink-based escape: an agent or a careless template could plant a symlink inside the work root that points to '/etc' (or any user file outside the project) and then read or write through the link believing the path was contained. Add a second pass that re-checks containment after `.resolvingSymlinksInPath()` on both the candidate path and the root, with proper trailing-separator handling so a sibling directory that happens to share a prefix (e.g. '/work/foo-baz' vs '/work/foo') can't bypass the prefix test. The work root itself is symlink-resolved before the comparison, so macOS-typical projects whose root is reached via a symlink (a common pattern, e.g. /var -> /private/var) don't fail spuriously. New focused tests cover: * relative path inside root accepted * '/path' (treated as relative) accepted * '..' escape rejected (pre-existing behavior, still covered) * symlink escape rejected (new) * root-via-symlink accepted (regression guard for the macOS pattern) Co-authored-by: Michael Meding --- .../WorkFolderToolsResolvePathTests.swift | 117 ++++++++++++++++++ .../OsaurusCore/Work/WorkFolderTools.swift | 36 +++++- 2 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 Packages/OsaurusCore/Tests/Work/WorkFolderToolsResolvePathTests.swift 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..5a42633ab 100644 --- a/Packages/OsaurusCore/Work/WorkFolderTools.swift +++ b/Packages/OsaurusCore/Work/WorkFolderTools.swift @@ -32,7 +32,24 @@ 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. After lexical `.standardized` normalization, which collapses `..` + /// segments and removes redundant `/./`. This catches the + /// overwhelmingly common case of an agent emitting `"../etc/passwd"`. + /// 2. After `.resolvingSymlinksInPath()`, which follows any symbolic + /// links along the resolved path. 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's contained. + /// + /// The work-root prefix used for the containment test is itself + /// symlink-resolved so the comparison is apples-to-apples — otherwise + /// a project whose root is itself reached through a symlink (a common + /// macOS pattern, e.g. `/var` → `/private/var`) would incorrectly + /// fail containment. static func resolvePath(_ relativePath: String, rootPath: URL) throws -> URL { let cleanPath = relativePath.hasPrefix("/") ? String(relativePath.dropFirst()) : relativePath let resolvedURL = rootPath.appendingPathComponent(cleanPath).standardized @@ -41,6 +58,23 @@ enum WorkFolderToolHelpers { guard resolvedURL.path.hasPrefix(rootPathString) else { throw WorkFolderToolError.pathOutsideRoot(relativePath) } + + // Symlink-aware re-check. Containment is enforced even if the + // resolved path traverses a symlink that exits the work root. + // Compare against a symlink-resolved root so projects whose root + // is itself reached through a symlink don't fail spuriously. + let symlinkResolvedURL = resolvedURL.resolvingSymlinksInPath() + let symlinkResolvedRoot = rootPath.resolvingSymlinksInPath().standardized.path + let rootWithSeparator = + symlinkResolvedRoot.hasSuffix("/") ? symlinkResolvedRoot : symlinkResolvedRoot + "/" + + let resolvedPath = symlinkResolvedURL.path + let isInside = + resolvedPath == symlinkResolvedRoot + || resolvedPath.hasPrefix(rootWithSeparator) + guard isInside else { + throw WorkFolderToolError.pathOutsideRoot(relativePath) + } return resolvedURL } From 48d0313ec93282a82ae047e0a143f2c0ea7054ee Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 27 May 2026 05:06:01 +0000 Subject: [PATCH 2/4] Path containment: compare canonical components, not string prefixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original fix used string-prefix matching ('resolvedURL.path.hasPrefix( rootPathString)') after only one side had been run through resolvingSymlinksInPath(). On macOS the runner's TempDirectory lives under /var/folders/... and the system selectively canonicalizes that prefix to /private/var/... depending on which API is used (FileManager.enumerator, URL.resolvingSymlinksInPath, URL.standardized — each applies a different rule). One side getting the /private prefix and the other not yielded false-negative containment failures and made the rejectsSymlinkEscape test fail in CI. Use pathComponents arrays + Array.starts(with:) after canonicalizing BOTH sides identically (resolvingSymlinksInPath().standardized). Component-based prefix matching also closes the 'sibling directory with shared name prefix' foot-gun (e.g. /work/foo-baz vs /work/foo) that the original trailing-separator string trick was trying to paper over. Co-authored-by: Michael Meding --- .../OsaurusCore/Work/WorkFolderTools.swift | 53 +++++++++---------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/Packages/OsaurusCore/Work/WorkFolderTools.swift b/Packages/OsaurusCore/Work/WorkFolderTools.swift index 5a42633ab..b7882df05 100644 --- a/Packages/OsaurusCore/Work/WorkFolderTools.swift +++ b/Packages/OsaurusCore/Work/WorkFolderTools.swift @@ -36,20 +36,25 @@ enum WorkFolderToolHelpers { /// /// The check runs twice: /// - /// 1. After lexical `.standardized` normalization, which collapses `..` - /// segments and removes redundant `/./`. This catches the - /// overwhelmingly common case of an agent emitting `"../etc/passwd"`. - /// 2. After `.resolvingSymlinksInPath()`, which follows any symbolic - /// links along the resolved path. 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's contained. + /// 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. After `.resolvingSymlinksInPath()`: follow symbolic links 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. /// - /// The work-root prefix used for the containment test is itself - /// symlink-resolved so the comparison is apples-to-apples — otherwise - /// a project whose root is itself reached through a symlink (a common - /// macOS pattern, e.g. `/var` → `/private/var`) would incorrectly - /// fail containment. + /// Containment is checked by *path components* after both sides are + /// run through `resolvingSymlinksInPath().standardized`. Pure-string + /// prefix matching breaks on macOS because the system canonicalizes + /// `/var` to `/private/var` only sometimes (the enumerator, the + /// symlink resolver, and `.standardized` each apply different rules + /// depending on whether the path was already in canonical form). Using + /// `Array.starts(with:)` over `pathComponents` avoids those mismatches + /// and also avoids the "sibling whose name shares a prefix" foot-gun + /// (e.g. `/work/foo-baz` vs `/work/foo`). static func resolvePath(_ relativePath: String, rootPath: URL) throws -> URL { let cleanPath = relativePath.hasPrefix("/") ? String(relativePath.dropFirst()) : relativePath let resolvedURL = rootPath.appendingPathComponent(cleanPath).standardized @@ -59,20 +64,14 @@ enum WorkFolderToolHelpers { throw WorkFolderToolError.pathOutsideRoot(relativePath) } - // Symlink-aware re-check. Containment is enforced even if the - // resolved path traverses a symlink that exits the work root. - // Compare against a symlink-resolved root so projects whose root - // is itself reached through a symlink don't fail spuriously. - let symlinkResolvedURL = resolvedURL.resolvingSymlinksInPath() - let symlinkResolvedRoot = rootPath.resolvingSymlinksInPath().standardized.path - let rootWithSeparator = - symlinkResolvedRoot.hasSuffix("/") ? symlinkResolvedRoot : symlinkResolvedRoot + "/" - - let resolvedPath = symlinkResolvedURL.path - let isInside = - resolvedPath == symlinkResolvedRoot - || resolvedPath.hasPrefix(rootWithSeparator) - guard isInside else { + // Symlink-aware re-check. Canonicalize both sides the same way + // (resolve symlinks, then standardize) so macOS's /var ↔ + // /private/var folder canonicalization can't produce a one-sided + // mismatch. Compare as `pathComponents` arrays to avoid the + // shared-prefix-substring bug. + let canonicalCandidate = resolvedURL.resolvingSymlinksInPath().standardized + let canonicalRoot = rootPath.resolvingSymlinksInPath().standardized + guard canonicalCandidate.pathComponents.starts(with: canonicalRoot.pathComponents) else { throw WorkFolderToolError.pathOutsideRoot(relativePath) } return resolvedURL From 72035c3395f5f23f61e558fee5becb1e1d6c0b9f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 27 May 2026 05:23:14 +0000 Subject: [PATCH 3/4] Walk path components for symlink containment (realpath -m semantics) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit macOS's URL.resolvingSymlinksInPath() uses strict-realpath semantics: if any component along the path doesn't currently exist, it returns the input URL UNCHANGED rather than resolving the components that do exist. In our case, an agent asking the tools to write '/root/escape/newfile' — where 'newfile' has not been created yet but 'escape' inside the work root IS a symlink pointing OUT of the root — would skate past the symlink-aware check because the resolver gave back the original unresolved string. The CI test 'rejectsSymlinkEscape' caught exactly this: 'secret' didn't exist, 'escape' was a symlink to a sibling temp directory, and resolvePath returned successfully. Walk the relative components manually. For each component, append it on top of the running resolved-so-far URL. If the resulting path exists on disk, run it through resolvingSymlinksInPath() so any symlink we encounter at this depth gets resolved. As soon as a component doesn't exist, stop resolving and append the rest of the path lexically. This is the moral equivalent of 'realpath -m' on Linux and gets us the right answer in the 'parent exists but leaf doesn't' case the tests exercise. Co-authored-by: Michael Meding --- .../OsaurusCore/Work/WorkFolderTools.swift | 77 ++++++++++++++----- 1 file changed, 57 insertions(+), 20 deletions(-) diff --git a/Packages/OsaurusCore/Work/WorkFolderTools.swift b/Packages/OsaurusCore/Work/WorkFolderTools.swift index b7882df05..57f729a31 100644 --- a/Packages/OsaurusCore/Work/WorkFolderTools.swift +++ b/Packages/OsaurusCore/Work/WorkFolderTools.swift @@ -40,21 +40,29 @@ enum WorkFolderToolHelpers { /// confirm the result still sits under `rootPath`. This catches the /// overwhelmingly common case of an agent emitting `"../etc/passwd"` /// without touching the filesystem. - /// 2. After `.resolvingSymlinksInPath()`: follow symbolic links 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. + /// 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. /// - /// Containment is checked by *path components* after both sides are - /// run through `resolvingSymlinksInPath().standardized`. Pure-string - /// prefix matching breaks on macOS because the system canonicalizes - /// `/var` to `/private/var` only sometimes (the enumerator, the - /// symlink resolver, and `.standardized` each apply different rules - /// depending on whether the path was already in canonical form). Using - /// `Array.starts(with:)` over `pathComponents` avoids those mismatches - /// and also avoids the "sibling whose name shares a prefix" foot-gun - /// (e.g. `/work/foo-baz` vs `/work/foo`). + /// 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 @@ -64,19 +72,48 @@ enum WorkFolderToolHelpers { throw WorkFolderToolError.pathOutsideRoot(relativePath) } - // Symlink-aware re-check. Canonicalize both sides the same way - // (resolve symlinks, then standardize) so macOS's /var ↔ - // /private/var folder canonicalization can't produce a one-sided - // mismatch. Compare as `pathComponents` arrays to avoid the - // shared-prefix-substring bug. - let canonicalCandidate = resolvedURL.resolvingSymlinksInPath().standardized + // 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), From 6fe5ee59778fd5d9a37edc43615edbdf37deece6 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 27 May 2026 05:08:34 +0000 Subject: [PATCH 4/4] Fix flake: skip ModelManager launch-time HF fetch under xctest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ModelManager.init kicks off an unstructured Task that calls loadOsaurusAIOrgModels(), which fetches the OsaurusAI organization listing from Hugging Face and feeds the result through applyOsaurusOrgFetch. The unit-test runner repeatedly constructs ModelManager() to drive applyOsaurusOrgFetch directly. The background launch-time fetch races with those test calls — whichever finishes last wins, and the merge result is non-deterministic. That's the root cause of the flaky ModelManagerSuggestedTests failures seen across many of the recent PR CI runs (applyOsaurusOrgFetch_dropsStaleAutoFetched OnReapply, applyOsaurusOrgFetch_addsNewEntriesAfterCurated, etc.). Gate the launch-time fetch on a small isRunningInTestEnvironment helper that checks for any of XCTestConfigurationFilePath, XCTestBundlePath, or XCTestSessionIdentifier in the process environment. Those variables are only present inside an xctest host process; production app launches still get the HF fetch exactly as before. This is a network call, so removing it under tests also has the side benefit of making the test suite work offline / on hermetic CI runners. Co-authored-by: Michael Meding --- .../Managers/Model/ModelManager.swift | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) 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