Skip to content
9 changes: 9 additions & 0 deletions scripts/verify-loop.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 16 additions & 2 deletions swiftpkg/BuildInfo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 `<name>.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,
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
Expand Down
40 changes: 38 additions & 2 deletions swiftpkg/PackageBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
}
Expand Down Expand Up @@ -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/<name>` (and `build/Dist-<name>` 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.
Expand All @@ -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.
Comment on lines +89 to +92

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add hermetic receipt-only compatibility coverage.

Add a Swift test that builds a project with neither payload nor scripts using TemporaryDirectory and RecordingRunner, and asserts the generated command uses the receipt-only path.

As per coding guidelines, behavior changes affecting “payload-free packages” require targeted compatibility tests, and unit tests must use TemporaryDirectory and RecordingRunner.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@swiftpkg/PackageBuilder.swift` around lines 89 - 92, Add targeted Swift
compatibility coverage for the receipt-only behavior in PackageBuilder: create a
project without payload or scripts using TemporaryDirectory and RecordingRunner,
build it, and assert the generated command uses the pkgbuild --nopayload path.
Keep the test hermetic and focused on payload-free package behavior.

Source: Coding guidelines

buildDirectory = outputDirectory ?? project.appendingPathComponent("build", isDirectory: true)
}

Expand Down Expand Up @@ -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") }
Expand Down Expand Up @@ -253,14 +279,24 @@ 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
}
}
}

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
}
2 changes: 1 addition & 1 deletion swiftpkg/PackageSettingsDraft.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public struct PackageSettingsDraft: Equatable, Sendable {
notarizationTeamID = ""
notarizationPassword = ""
notarizationKeychainProfile = profile
case nil:
case nil, .invalid?:
notarizationMode = .none
notarizationAppleID = ""
notarizationTeamID = ""
Expand Down
4 changes: 4 additions & 0 deletions swiftpkgCLI/CLI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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/.
"""
Expand Down
14 changes: 14 additions & 0 deletions swiftpkgTests/CLITests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
30 changes: 30 additions & 0 deletions swiftpkgTests/KeychainPathTests.swift
Original file line number Diff line number Diff line change
@@ -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")
}
}
43 changes: 43 additions & 0 deletions swiftpkgTests/NotarizationDeferTests.swift
Original file line number Diff line number Diff line change
@@ -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
}
}
}
32 changes: 32 additions & 0 deletions swiftpkgTests/PackageNameExtensionTests.swift
Original file line number Diff line number Diff line change
@@ -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")
}
}
53 changes: 53 additions & 0 deletions swiftpkgTests/PackageNameValidationTests.swift
Original file line number Diff line number Diff line change
@@ -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)
}
}
30 changes: 30 additions & 0 deletions swiftpkgTests/ReceiptOnlyBuildTests.swift
Original file line number Diff line number Diff line change
@@ -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"))
}
}