diff --git a/scripts/verify-loop.sh b/scripts/verify-loop.sh index 42b7ba6..1e949b1 100755 --- a/scripts/verify-loop.sh +++ b/scripts/verify-loop.sh @@ -58,6 +58,15 @@ run "$BIN" "$EMPTY" run /usr/sbin/pkgutil --expand "$EMPTY/build/EmptyPayload-1.0.pkg" "$WORK/expanded-empty" test -e "$WORK/expanded-empty/Payload" +RECEIPT="$WORK/ReceiptOnly" +run "$BIN" --create "$RECEIPT" +rm -rf "$RECEIPT/payload" "$RECEIPT/scripts" +run "$BIN" "$RECEIPT" +run /usr/sbin/pkgutil --expand "$RECEIPT/build/ReceiptOnly-1.0.pkg" "$WORK/expanded-receipt" +test ! -e "$WORK/expanded-receipt/Payload" +test ! -e "$WORK/expanded-receipt/Scripts" +printf 'receipt-only package OK\n' + for format in json yaml; do PROJECT_FORMAT="$WORK/Format-$format" if [ "$format" = json ]; then diff --git a/swiftpkg/BuildInfo.swift b/swiftpkg/BuildInfo.swift index ee04861..1044266 100644 --- a/swiftpkg/BuildInfo.swift +++ b/swiftpkg/BuildInfo.swift @@ -49,6 +49,10 @@ public struct NotarizationConfiguration: Sendable { public enum Authentication: Sendable { case appleID(appleID: String, teamID: String, password: String) case keychainProfile(String) + /// notarization_info was present but incomplete. Loading tolerates this + /// so `--skip-notarization` builds still succeed; the error surfaces only + /// if notarization is actually attempted. + case invalid(reason: String) } public let authentication: Authentication @@ -154,12 +158,18 @@ public struct PackageConfiguration: Sendable { } /// Returns a copy with `${version}` substituted in user-facing name fields. + /// The resolved package name is normalized to end in `.pkg`: build-info may + /// set `name` without the extension (e.g. `MunkiBootstrap`), and munki-pkg + /// writes the artifact as `.pkg`, so swiftpkg does too — otherwise the + /// output is extensionless and `find '*.pkg'`/munkiimport miss it. public func substitutingVersion() -> PackageConfiguration { func replacingVersion(in value: String?) -> String? { value?.replacingOccurrences(of: "${version}", with: version) } + let resolvedName = replacingVersion(in: name)! + let normalizedName = resolvedName.hasSuffix(".pkg") ? resolvedName : "\(resolvedName).pkg" return PackageConfiguration( - name: replacingVersion(in: name)!, identifier: identifier, version: version, ownership: ownership, + name: normalizedName, identifier: identifier, version: version, ownership: ownership, installLocation: installLocation, compression: compression, minimumOSVersion: minimumOSVersion, usesLargePayload: usesLargePayload, postInstallAction: postInstallAction, preservesExtendedAttributes: preservesExtendedAttributes, suppressesBundleRelocation: suppressesBundleRelocation, @@ -227,7 +237,10 @@ public struct PackageConfiguration: Sendable { } else if let profile = notary["keychain_profile"] as? String { authentication = .keychainProfile(profile) } else { - throw MunkiPkgError.invalidConfiguration("notarization_info must specify apple_id + team_id or keychain_profile") + // Tolerate incomplete notarization_info at load time so + // --skip-notarization builds succeed. munki-pkg defers this check to + // the point notarization actually runs; so does swiftpkg. + authentication = .invalid(reason: "notarization_info must specify apple_id + team_id or keychain_profile") } let timeout = (notary["staple_timeout"] as? NSNumber)?.intValue ?? 300 return NotarizationConfiguration(authentication: authentication, staplingTimeout: timeout) @@ -252,6 +265,7 @@ private extension NotarizationConfiguration { case let .appleID(appleID, teamID, password): values.merge(["apple_id": appleID, "team_id": teamID, "password": password]) { _, new in new } case let .keychainProfile(profile): values["keychain_profile"] = profile + case .invalid: break } return values } diff --git a/swiftpkg/PackageBuilder.swift b/swiftpkg/PackageBuilder.swift index d6e10ec..8757ee9 100644 --- a/swiftpkg/PackageBuilder.swift +++ b/swiftpkg/PackageBuilder.swift @@ -15,6 +15,7 @@ public struct PackageBuildCoordinator: @unchecked Sendable { public func buildPackage(in project: URL, configuration: PackageBuildOptions) async throws { let packageConfiguration = try BuildInfoStore.load(from: project, requestedFormat: configuration.requestedFormat, versionOverride: configuration.versionOverride) + try Self.validatePackageName(packageConfiguration.name) if packageConfiguration.ownership != .recommended, geteuid() != 0 { console.warning("build-info ownership: \(packageConfiguration.ownership.rawValue) might require using sudo to build this package.") } @@ -47,6 +48,25 @@ public struct PackageBuildCoordinator: @unchecked Sendable { } return URL(fileURLWithPath: String(path)) } + + /// Rejects a package `name` that could escape the build directory. + /// + /// The output package path is `build/` (and `build/Dist-` for + /// distribution builds), so a `name` containing a path separator or `..` + /// would write the artifact outside `build/`. Require a single, safe path + /// component. Called with the post-`${version}`-substitution name. + static func validatePackageName(_ name: String) throws { + guard !name.isEmpty, + name != ".", name != "..", + !name.contains("/"), + !name.contains("\0"), + URL(fileURLWithPath: name).lastPathComponent == name + else { + throw MunkiPkgError.invalidConfiguration( + "Package name \"\(name)\" must be a single path component (no \"/\" or \"..\")." + ) + } + } } /// Describes the files and directories involved in one package build. @@ -66,7 +86,10 @@ private struct PackageProjectLayout { if fileManager.directoryExists(at: scriptsURL), try !fileManager.contents(at: scriptsURL).filter({ $0 != ".DS_Store" }).isEmpty { scripts = scriptsURL } else { scripts = nil } - guard payload != nil || scripts != nil else { throw MunkiPkgError.message("\(project.path) does not contain a payload folder or a scripts folder.") } + // A project with neither payload nor scripts is valid: it builds a + // receipt-only package (pkgbuild --nopayload) that installs no files but + // records a receipt, which Munki conditions can key off. munki-pkg + // allows this, so swiftpkg does too. buildDirectory = outputDirectory ?? project.appendingPathComponent("build", isDirectory: true) } @@ -198,6 +221,9 @@ struct NotarizationService: Sendable { let console: Console func notarize(package: URL, configuration: NotarizationConfiguration, skipsStapling: Bool) async throws { + if case let .invalid(reason) = configuration.authentication { + throw MunkiPkgError.invalidConfiguration(reason) + } console.display("Uploading package to Apple notary service") let submission = try plistOutput(for: ["notarytool", "submit", "--output-format", "plist", package.path] + authenticationArguments(for: configuration), failureMessage: "Notarization upload failed.") guard let identifier = submission["id"] as? String else { throw MunkiPkgError.notarizationFailed("Unexpected output from notarytool") } @@ -253,6 +279,7 @@ struct NotarizationService: Sendable { switch configuration.authentication { case let .appleID(appleID, teamID, password): return ["--apple-id", appleID, "--team-id", teamID, "--password", password] case let .keychainProfile(profile): return ["--keychain-profile", profile] + case .invalid: return [] // notarize(package:...) rejects .invalid before reaching here } } } @@ -260,7 +287,16 @@ struct NotarizationService: Sendable { private func appendSigningArguments(_ arguments: inout [String], signing: SigningConfiguration?) { guard let signing else { return } arguments += ["--sign", signing.identity] - if let keychain = signing.keychain { arguments += ["--keychain", keychain] } + if let keychain = signing.keychain { arguments += ["--keychain", expandKeychainPath(keychain)] } for certificate in signing.additionalCertificateNames { arguments += ["--cert", certificate] } if let usesTimestamp = signing.usesTimestamp { arguments.append(usesTimestamp ? "--timestamp" : "--timestamp=none") } } + +/// Expands `${HOME}` and a leading tilde in a build-info keychain path so that +/// projects written as `${HOME}/Library/Keychains/signing.keychain` resolve to a +/// real path before being handed to `productbuild`/`productsign`. Mirrors the +/// original munki-pkg, whose build-info files rely on this expansion. +func expandKeychainPath(_ path: String) -> String { + let withHome = path.replacingOccurrences(of: "${HOME}", with: NSHomeDirectory()) + return NSString(string: withHome).expandingTildeInPath +} diff --git a/swiftpkg/PackageSettingsDraft.swift b/swiftpkg/PackageSettingsDraft.swift index cca82b1..6d143ac 100644 --- a/swiftpkg/PackageSettingsDraft.swift +++ b/swiftpkg/PackageSettingsDraft.swift @@ -66,7 +66,7 @@ public struct PackageSettingsDraft: Equatable, Sendable { notarizationTeamID = "" notarizationPassword = "" notarizationKeychainProfile = profile - case nil: + case nil, .invalid?: notarizationMode = .none notarizationAppleID = "" notarizationTeamID = "" diff --git a/swiftpkgCLI/CLI.swift b/swiftpkgCLI/CLI.swift index d2144a4..3ba96e4 100644 --- a/swiftpkgCLI/CLI.swift +++ b/swiftpkgCLI/CLI.swift @@ -36,6 +36,9 @@ public struct CLIOptions: ParsableArguments { @Flag(name: .long, help: "Skip stapling after notarization.") public var skipStapling = false + @Flag(name: .long, help: "Accepted for munki-pkg compatibility; ignored. swiftpkg never prompts to import into a repo, so there is nothing to skip.") + public var skipImport = false + @Flag(name: .long, help: "Show program's version number and exit.") public var version = false @@ -127,6 +130,7 @@ public enum CLIParser { --skip-signing Skip configured package signing. --skip-notarization Skip configured notarization. --skip-stapling Skip stapling after notarization. + --skip-import Accepted for munki-pkg compatibility; ignored. --pkg-version VERSION Override the build-info version. --output-dir DIR Write the package to DIR instead of build/. """ diff --git a/swiftpkgTests/CLITests.swift b/swiftpkgTests/CLITests.swift index feed2cd..0593c2f 100644 --- a/swiftpkgTests/CLITests.swift +++ b/swiftpkgTests/CLITests.swift @@ -44,6 +44,20 @@ struct CLITests { #expect(options.importPackage == "existing.pkg") } + @Test("accepts --skip-import as an ignored no-op and still builds") + func acceptsSkipImport() throws { + guard case .options(let options) = CLIParser.parse(["--skip-import", "Project"]) else { + Issue.record("Expected options result") + return + } + #expect(options.skipImport) + // The flag must not divert away from a normal build. + guard case .build = try #require(try CLICommand.resolve(from: options)) else { + Issue.record("Expected a build command") + return + } + } + @Test("reports missing import argument") func reportsMissingImportArgument() { guard case .failure(let message) = CLIParser.parse(["--import"]) else { diff --git a/swiftpkgTests/KeychainPathTests.swift b/swiftpkgTests/KeychainPathTests.swift new file mode 100644 index 0000000..3f02462 --- /dev/null +++ b/swiftpkgTests/KeychainPathTests.swift @@ -0,0 +1,30 @@ +import Foundation +import Testing +@testable import SwiftPkgCore + +struct KeychainPathTests { + + @Test("expands ${HOME} to the user's home directory") + func expandsHome() { + let expanded = expandKeychainPath("${HOME}/Library/Keychains/signing.keychain") + #expect(expanded == "\(NSHomeDirectory())/Library/Keychains/signing.keychain") + #expect(!expanded.contains("${HOME}")) + } + + @Test("expands a leading tilde") + func expandsTilde() { + let expanded = expandKeychainPath("~/Library/Keychains/signing.keychain") + #expect(expanded == "\(NSHomeDirectory())/Library/Keychains/signing.keychain") + } + + @Test("leaves an absolute path unchanged") + func leavesAbsolutePathUnchanged() { + let path = "/Library/Keychains/System.keychain" + #expect(expandKeychainPath(path) == path) + } + + @Test("leaves a bare keychain name unchanged") + func leavesBareNameUnchanged() { + #expect(expandKeychainPath("login.keychain") == "login.keychain") + } +} diff --git a/swiftpkgTests/NotarizationDeferTests.swift b/swiftpkgTests/NotarizationDeferTests.swift new file mode 100644 index 0000000..e6f777f --- /dev/null +++ b/swiftpkgTests/NotarizationDeferTests.swift @@ -0,0 +1,43 @@ +import Foundation +import Testing +@testable import SwiftPkgCore + +struct NotarizationDeferTests { + private var defaults: PackageConfiguration { .defaults(for: URL(fileURLWithPath: "/tmp/Proj")) } + + @Test("incomplete notarization_info loads as .invalid instead of throwing") + func incompleteLoadsAsInvalid() throws { + let values: [String: Any] = [ + "name": "Proj.pkg", "identifier": "com.example.proj", "version": "1.0", + "notarization_info": ["password": "abcd-efgh-ijkl-mnop"] + ] + let config = try PackageConfiguration(values: values, defaults: defaults) + guard case .invalid = try #require(config.notarization?.authentication) else { + Issue.record("Expected .invalid authentication") + return + } + } + + @Test("complete notarization_info still parses to a usable authentication") + func completeParses() throws { + let profileValues: [String: Any] = [ + "name": "Proj.pkg", "identifier": "com.example.proj", "version": "1.0", + "notarization_info": ["keychain_profile": "notary"] + ] + let profile = try PackageConfiguration(values: profileValues, defaults: defaults) + guard case .keychainProfile("notary") = try #require(profile.notarization?.authentication) else { + Issue.record("Expected .keychainProfile") + return + } + + let appleValues: [String: Any] = [ + "name": "Proj.pkg", "identifier": "com.example.proj", "version": "1.0", + "notarization_info": ["apple_id": "a@b.com", "team_id": "TEAM"] + ] + let apple = try PackageConfiguration(values: appleValues, defaults: defaults) + guard case .appleID = try #require(apple.notarization?.authentication) else { + Issue.record("Expected .appleID") + return + } + } +} diff --git a/swiftpkgTests/PackageNameExtensionTests.swift b/swiftpkgTests/PackageNameExtensionTests.swift new file mode 100644 index 0000000..9715b5b --- /dev/null +++ b/swiftpkgTests/PackageNameExtensionTests.swift @@ -0,0 +1,32 @@ +import Foundation +import Testing +@testable import SwiftPkgCore + +struct PackageNameExtensionTests { + private var defaults: PackageConfiguration { .defaults(for: URL(fileURLWithPath: "/tmp/Proj")) } + + private func config(name: String, version: String = "1.0") throws -> PackageConfiguration { + try PackageConfiguration( + values: ["name": name, "identifier": "com.example.proj", "version": version], + defaults: defaults + ) + } + + @Test("a name without .pkg gains the extension") + func appendsExtension() throws { + let resolved = try config(name: "MunkiBootstrap").substitutingVersion() + #expect(resolved.name == "MunkiBootstrap.pkg") + } + + @Test("a name already ending in .pkg is left unchanged") + func keepsExtension() throws { + let resolved = try config(name: "AdminDock.pkg").substitutingVersion() + #expect(resolved.name == "AdminDock.pkg") + } + + @Test("the extension is appended after ${version} substitution") + func appendsAfterVersionSubstitution() throws { + let resolved = try config(name: "Tool-${version}", version: "2.3").substitutingVersion() + #expect(resolved.name == "Tool-2.3.pkg") + } +} diff --git a/swiftpkgTests/PackageNameValidationTests.swift b/swiftpkgTests/PackageNameValidationTests.swift new file mode 100644 index 0000000..ff39e05 --- /dev/null +++ b/swiftpkgTests/PackageNameValidationTests.swift @@ -0,0 +1,53 @@ +import Foundation +import Testing +@testable import SwiftPkgCore + +struct PackageNameValidationTests { + + @Test("rejects names that escape the build directory", arguments: [ + "../evil.pkg", + "../../tmp/evil.pkg", + "sub/dir/evil.pkg", + "/etc/evil.pkg", + "..", + ".", + "", + ]) + func rejectsUnsafeNames(_ name: String) { + #expect(throws: MunkiPkgError.self) { + try PackageBuildCoordinator.validatePackageName(name) + } + } + + @Test("accepts safe single-component names", arguments: [ + "MyApp-1.0.pkg", + "com.example.thing.pkg", + "package_2026.07.18.pkg", + "no-extension", + ]) + func acceptsSafeNames(_ name: String) throws { + try PackageBuildCoordinator.validatePackageName(name) + } + + // End-to-end: a malicious build-info name must fail before pkgbuild runs. + @Test("build with a traversal name throws and never invokes pkgbuild") + func buildRejectsTraversalNameBeforeRunning() async throws { + let temp = try TemporaryDirectory() + defer { temp.remove() } + let project = temp.url.appendingPathComponent("Evil", isDirectory: true) + let payload = project.appendingPathComponent("payload", isDirectory: true) + try FileManager.default.createDirectory(at: payload, withIntermediateDirectories: true) + try write("payload", to: payload.appendingPathComponent("file.txt")) + try write( + #"{"name":"../evil.pkg","identifier":"com.test.evil","version":"1.0"}"#, + to: project.appendingPathComponent("build-info.json") + ) + + let runner = RecordingRunner() + let coordinator = PackageBuildCoordinator(fileManager: .default, runner: runner, console: makeConsole()) + await #expect(throws: MunkiPkgError.self) { + try await coordinator.buildPackage(in: project, configuration: PackageBuildOptions()) + } + #expect(runner.calls.isEmpty) + } +} diff --git a/swiftpkgTests/ReceiptOnlyBuildTests.swift b/swiftpkgTests/ReceiptOnlyBuildTests.swift new file mode 100644 index 0000000..254a0f0 --- /dev/null +++ b/swiftpkgTests/ReceiptOnlyBuildTests.swift @@ -0,0 +1,30 @@ +import Foundation +import Testing +@testable import SwiftPkgCore + +struct ReceiptOnlyBuildTests { + + // A project with neither payload nor scripts is valid: it builds a + // receipt-only package. pkgbuild must be invoked with --nopayload (and no + // --root), matching munki-pkg — otherwise the build would fail looking for + // a payload that isn't there. + @Test("receipt-only project builds with pkgbuild --nopayload") + func receiptOnlyUsesNopayload() async throws { + let temp = try TemporaryDirectory() + defer { temp.remove() } + let project = temp.url.appendingPathComponent("ReceiptOnly", isDirectory: true) + try FileManager.default.createDirectory(at: project, withIntermediateDirectories: true) + try write( + #"{"name":"ReceiptOnly-1.0.pkg","identifier":"com.test.receipt","version":"1.0"}"#, + to: project.appendingPathComponent("build-info.json") + ) + + let runner = RecordingRunner() + let coordinator = PackageBuildCoordinator(fileManager: .default, runner: runner, console: makeConsole()) + try await coordinator.buildPackage(in: project, configuration: PackageBuildOptions(skipsSigning: true)) + + let pkgbuild = try #require(runner.calls.first { $0.executable.hasSuffix("pkgbuild") }) + #expect(pkgbuild.arguments.contains("--nopayload")) + #expect(!pkgbuild.arguments.contains("--root")) + } +}