diff --git a/Sources/RunnerCore/PathResolution.swift b/Sources/RunnerCore/PathResolution.swift new file mode 100644 index 0000000..e230db7 --- /dev/null +++ b/Sources/RunnerCore/PathResolution.swift @@ -0,0 +1,110 @@ +import Foundation + +#if canImport(Darwin) + import Darwin +#elseif canImport(Glibc) + import Glibc +#endif + +/// Resolves a path typed on the command line into a single concrete path. +/// +/// The problem this exists for: an operator writes +/// `--ipsw ~/Downloads/UniversalMac_27.0_*.ipsw`. Unquoted, the shell may expand +/// the glob before we ever run — or, in `zsh`, refuse to run the command at all +/// with `no matches found`. Quoted, we receive the pattern verbatim, tilde and +/// asterisk included, and a plain `expandingTildeInPath` leaves a `*` sitting in +/// the middle of a path that will never exist. Either way the operator sees a +/// tool that "only works with quotes" (or only without them). +/// +/// So the CLI resolves the argument itself and stops depending on which shell +/// ran it. This is deliberately a **CLI-argument affordance only**: values read +/// out of `config.json` get tilde expansion (see +/// ``RunnerConfig/expandTilde(_:)``) and nothing more, because a config file is +/// not typed at a prompt and a stray `*` there is a mistake, not a pattern. +public enum PathResolution { + + /// Characters that make a string a pattern rather than a path. + /// + /// `~` is not one of them: it is expanded unconditionally, pattern or not. + private static let metacharacters: Set = ["*", "?", "["] + + /// Whether `path` should be treated as a glob pattern. + public static func isPattern(_ path: String) -> Bool { + path.contains(where: metacharacters.contains) + } + + /// Expands a leading `~`, then resolves any glob pattern to one path. + /// + /// A string with no metacharacters passes through with only tilde + /// expansion — in particular it is *not* checked for existence, so the + /// caller's own error (which knows what the file was for) is what an + /// operator sees for an ordinary typo. + /// + /// A string that does contain metacharacters is matched with `glob(3)`. If + /// it matches nothing but a file exists at that literal name, the literal + /// wins: `report[1].ipsw` is a legal filename, and only failing to match + /// tells us it was meant as one rather than as a character class. + /// + /// - Parameters: + /// - path: The raw argument, as typed. + /// - label: What the argument names, for error messages (`"--ipsw"`). + /// - Returns: A single concrete path. + /// - Throws: ``CoreError/notFound(_:)`` when a pattern matches nothing, or + /// ``CoreError/configInvalid(_:)`` when it matches more than one file — + /// picking one arbitrarily would silently build the wrong image. + public static func resolve(_ path: String, label: String) throws -> String { + let expanded = RunnerConfig.expandTilde(path) + guard isPattern(expanded) else { return expanded } + + let matches = glob(pattern: expanded) + + switch matches.count { + case 1: + return matches[0] + case 0: + if FileManager.default.fileExists(atPath: expanded) { return expanded } + throw CoreError.notFound("\(label): no file matches \(expanded)") + default: + let listed = matches.map { " \($0)" }.joined(separator: "\n") + throw CoreError.configInvalid( + "\(label): \(matches.count) files match \(expanded):\n\(listed)\n" + + "name exactly one of them" + ) + } + } + + /// All paths matching `pattern`, sorted. + /// + /// Sorted explicitly rather than relying on `glob(3)`'s own ordering, which + /// is locale-dependent — the error message above lists these, and a listing + /// that reorders between runs is a poor thing to ask someone to read. + static func glob(pattern: String) -> [String] { + var result = glob_t() + defer { globfree(&result) } + + guard Glibc_glob(pattern, &result) == 0 else { return [] } + guard let paths = result.gl_pathv else { return [] } + + var found: [String] = [] + for index in 0.. + ) -> Int32 { + #if canImport(Darwin) + return Darwin.glob(pattern, 0, nil, result) + #elseif canImport(Glibc) + return Glibc.glob(pattern, 0, nil, result) + #else + return -1 + #endif + } +} diff --git a/Sources/gitea-macos-runner/CommandImage.swift b/Sources/gitea-macos-runner/CommandImage.swift index 472d54e..b681fa8 100644 --- a/Sources/gitea-macos-runner/CommandImage.swift +++ b/Sources/gitea-macos-runner/CommandImage.swift @@ -35,7 +35,13 @@ struct ImageCommand: AsyncParsableCommand { var name: String = "default" /// A local `.ipsw`; omit to download the latest supported image. - @Option(name: .long, help: "Path to a local .ipsw (default: download the latest supported).") + /// + /// Resolved through ``PathResolution`` so it works whether or not the + /// shell got to the glob first: `--ipsw '~/Downloads/UniversalMac_27*.ipsw'` + /// and the unquoted form both land on the same file. + @Option( + name: .long, + help: "Path to a local .ipsw; may be a glob (default: download the latest supported).") var ipsw: String? /// Nominal guest disk size, overriding `guest.diskGB`. @@ -44,6 +50,12 @@ struct ImageCommand: AsyncParsableCommand { func run() async throws { CLI.bootstrapLogging(verbose: options.verbose) + + // Before anything else touches the disk: an unresolvable --ipsw is + // an argument error, and an argument error should not first make the + // operator wait on a config load and a free-space check. + let resolvedIPSW = try ipsw.map { try PathResolution.resolve($0, label: "--ipsw") } + var config = try options.loadConfig() if let diskGB { config.guest.diskGB = diskGB @@ -64,7 +76,7 @@ struct ImageCommand: AsyncParsableCommand { let printer = ProgressPrinter() let builder = ImageBuilder(store: store) let imageName = name - let ipswPath = ipsw + let ipswPath = resolvedIPSW let frozenConfig = config // `image build` runs `VZMacOSInstaller` and then boots the guest, so @@ -201,20 +213,28 @@ struct ImageCommand: AsyncParsableCommand { /// Optional Xcode `.xip` to install into the guest. Adds tens of /// gigabytes; omitted by default. - @Option(name: .customLong("xcode-xip"), help: "Path to an Xcode .xip to install into the guest.") + @Option( + name: .customLong("xcode-xip"), + help: "Path to an Xcode .xip to install into the guest; may be a glob.") var xcodeXIP: String? func run() async throws { CLI.bootstrapLogging(verbose: options.verbose) + + // Same treatment as `image build --ipsw`, and for the same reason: + // this is a long path to a big file that people reach for with a + // glob. Resolved first so a bad one costs nothing. + let resolvedXIP = try xcodeXIP.map { try PathResolution.resolve($0, label: "--xcode-xip") } + if let resolvedXIP, !FileManager.default.fileExists(atPath: resolvedXIP) { + throw ValidationError("no file at \(resolvedXIP)") + } + let config = try options.loadConfig() let store = VMStore(config: config) guard try store.image(named: name) != nil else { throw ValidationError("no image named '\(name)'") } - if let xcodeXIP, !FileManager.default.fileExists(atPath: RunnerConfig.expandTilde(xcodeXIP)) { - throw ValidationError("no file at \(RunnerConfig.expandTilde(xcodeXIP))") - } CLI.note("provisioning base image '\(name)' in place — stop the daemon before doing this") @@ -222,7 +242,7 @@ struct ImageCommand: AsyncParsableCommand { let builder = ImageBuilder(store: store) let imageName = name let frozenConfig = config - let xipPath = xcodeXIP.map(RunnerConfig.expandTilde) + let xipPath = resolvedXIP // Boots the image to run provision.sh in it, so it needs the run // loop for exactly the reason `image build` does. diff --git a/Tests/RunnerCoreTests/PathResolutionTests.swift b/Tests/RunnerCoreTests/PathResolutionTests.swift new file mode 100644 index 0000000..343ad2f --- /dev/null +++ b/Tests/RunnerCoreTests/PathResolutionTests.swift @@ -0,0 +1,154 @@ +import Foundation +import Testing + +@testable import RunnerCore + +/// Tests for ``PathResolution`` — the CLI's defence against shell quoting. +/// +/// The bug these exist for: `--ipsw ~/Downloads/UniversalMac_27.0_*.ipsw` +/// behaves differently depending on whether the shell expanded the glob, so the +/// tool has to resolve the argument itself and stop caring. +@Suite("PathResolution") +struct PathResolutionTests { + + /// A scratch directory holding `names`, deleted when `body` returns. + private func withFiles(_ names: [String], _ body: (String) throws -> Void) throws { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("gmr-path-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: dir) } + + for name in names { + let path = dir.appendingPathComponent(name) + #expect(FileManager.default.createFile(atPath: path.path, contents: Data())) + } + try body(dir.path) + } + + // MARK: - Pattern detection + + @Test("only glob metacharacters make a string a pattern") + func patternDetection() { + #expect(!PathResolution.isPattern("/tmp/UniversalMac_27.0.ipsw")) + // A tilde is expanded either way; it does not make this a glob. + #expect(!PathResolution.isPattern("~/Downloads/x.ipsw")) + #expect(PathResolution.isPattern("/tmp/a_*.ipsw")) + #expect(PathResolution.isPattern("/tmp/a_?.ipsw")) + #expect(PathResolution.isPattern("/tmp/a_[12].ipsw")) + } + + // MARK: - Passthrough + + @Test("a plain path is returned untouched") + func plainPathPassesThrough() throws { + // Deliberately not checked for existence: the caller's own error knows + // what the file was for and says something more useful than we could. + #expect(try PathResolution.resolve("/tmp/nope.ipsw", label: "--ipsw") == "/tmp/nope.ipsw") + #expect(try PathResolution.resolve("relative/x.ipsw", label: "--ipsw") == "relative/x.ipsw") + } + + @Test("a leading tilde is expanded") + func tildeIsExpanded() throws { + let home = NSHomeDirectory() + #expect(try PathResolution.resolve("~/Downloads/x.ipsw", label: "--ipsw") == home + "/Downloads/x.ipsw") + // Only leading: a tilde inside a path component is an ordinary character. + #expect(try PathResolution.resolve("/tmp/~x.ipsw", label: "--ipsw") == "/tmp/~x.ipsw") + } + + // MARK: - Globbing + + @Test("a pattern matching exactly one file resolves to it") + func singleMatchResolves() throws { + try withFiles(["UniversalMac_27.0_ABC.ipsw", "notes.txt"]) { dir in + let resolved = try PathResolution.resolve("\(dir)/UniversalMac_27.0_*.ipsw", label: "--ipsw") + #expect(resolved == "\(dir)/UniversalMac_27.0_ABC.ipsw") + } + } + + @Test("a pattern matching nothing is a clear notFound") + func zeroMatchesThrows() throws { + try withFiles(["a_1.ipsw"]) { dir in + #expect(throws: CoreError.self) { + try PathResolution.resolve("\(dir)/nope_*.ipsw", label: "--ipsw") + } + do { + _ = try PathResolution.resolve("\(dir)/nope_*.ipsw", label: "--ipsw") + Issue.record("expected a throw") + } catch let error as CoreError { + guard case .notFound(let message) = error else { + Issue.record("expected .notFound, got \(error)") + return + } + #expect(message.contains("--ipsw")) + #expect(message.contains("no file matches")) + #expect(message.contains("\(dir)/nope_*.ipsw")) + } + } + } + + @Test("a pattern matching several files lists them, sorted, and refuses") + func multipleMatchesThrowsAndLists() throws { + // Created out of order: the message must not depend on creation order, + // because the operator is being asked to read it and pick one. + try withFiles(["a_2.ipsw", "a_10.ipsw", "a_1.ipsw"]) { dir in + do { + _ = try PathResolution.resolve("\(dir)/a_*.ipsw", label: "--ipsw") + Issue.record("expected a throw") + } catch let error as CoreError { + guard case .configInvalid(let message) = error else { + Issue.record("expected .configInvalid, got \(error)") + return + } + #expect(message.contains("--ipsw")) + #expect(message.contains("3 files match")) + for name in ["a_1.ipsw", "a_2.ipsw", "a_10.ipsw"] { + #expect(message.contains("\(dir)/\(name)")) + } + // Sorted, so two runs read identically. + let one = message.range(of: "a_1.ipsw")!.lowerBound + let ten = message.range(of: "a_10.ipsw")!.lowerBound + let two = message.range(of: "a_2.ipsw")!.lowerBound + #expect(one < ten) + #expect(ten < two) + #expect(message.contains("name exactly one of them")) + } + } + } + + @Test("brackets are a character class when they match, and a filename when they do not") + func bracketsAreGlobOnlyWhenTheyAreOne() throws { + // Used as a class: `[12]` selects the one file that exists. + try withFiles(["build_1.ipsw"]) { dir in + let asClass = try PathResolution.resolve("\(dir)/build_[12].ipsw", label: "--ipsw") + #expect(asClass == "\(dir)/build_1.ipsw") + } + + // A real file whose name contains brackets. Read as a class it matches + // nothing, so the literal has to win — otherwise a legal filename is + // unreachable through this flag. + try withFiles(["report[1].ipsw"]) { dir in + let asLiteral = try PathResolution.resolve("\(dir)/report[1].ipsw", label: "--ipsw") + #expect(asLiteral == "\(dir)/report[1].ipsw") + } + } + + @Test("a pattern that resolves is not confused by neighbours of another extension") + func matchingIsScopedToThePattern() throws { + try withFiles(["a_1.ipsw", "a_1.ipsw.part", "a_1.txt"]) { dir in + let resolved = try PathResolution.resolve("\(dir)/a_*.ipsw", label: "--ipsw") + #expect(resolved == "\(dir)/a_1.ipsw") + } + } + + @Test("the label names the offending flag so the operator knows what to fix") + func labelAppearsInErrors() throws { + try withFiles([]) { dir in + do { + _ = try PathResolution.resolve("\(dir)/*.xip", label: "--xcode-xip") + Issue.record("expected a throw") + } catch let error as CoreError { + #expect("\(error)".contains("--xcode-xip")) + } + } + } +} diff --git a/docs/setup.md b/docs/setup.md index 747e345..5bf573f 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -315,6 +315,17 @@ find it. `--ipsw` is optional: omit it and the latest supported restore image is downloaded into `storeDir/ipsw/` first, which is most of the build's wall-clock time. `--disk-gb` overrides `guest.diskGB` for this image only. +`--ipsw` (and `image provision --xcode-xip`) accept a `~` and a glob, quoted or not — these are +equivalent, and neither depends on what your shell did with the pattern first: + +```sh +gitea-macos-runner image build --ipsw ~/Downloads/UniversalMac_27.0_*.ipsw +gitea-macos-runner image build --ipsw '~/Downloads/UniversalMac_27.0_*.ipsw' +``` + +A pattern must identify exactly one file. If it matches several, the build stops before doing any +work and lists them so you can name the one you meant; if it matches none, it says so. + This takes a long time — macOS installs from the IPSW, boots, and is then provisioned over SSH. **Do not interrupt it during the install phase.** Stopping a VM mid-install leaves the disk image in an undefined state; delete the image and start over rather than trying to resume.