-
Notifications
You must be signed in to change notification settings - Fork 1
Add --lint to validate a project without building #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
7e21571
Add --lint to validate a project without building
rodchristiansen 30c31d6
Harden lint: validate reverse-DNS components and reject script direct…
rodchristiansen deb2915
Lint the bad-script case without a payload
rodchristiansen a85e3a8
Merge next into feat/lint
jordancalhoun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| import Foundation | ||
|
|
||
| /// One problem found by `--lint`. | ||
| public struct LintFinding: Sendable, Equatable { | ||
| public enum Severity: String, Sendable { | ||
| case error | ||
| case warning | ||
| } | ||
|
|
||
| public let severity: Severity | ||
| public let message: String | ||
|
|
||
| public init(_ severity: Severity, _ message: String) { | ||
| self.severity = severity | ||
| self.message = message | ||
| } | ||
| } | ||
|
|
||
| /// Validates a package project without building it, for fast PR/CI pre-checks. | ||
| public struct Linter { | ||
| private let fileManager: FileManager | ||
|
|
||
| public init(fileManager: FileManager = .default) { | ||
| self.fileManager = fileManager | ||
| } | ||
|
|
||
| /// Returns findings. A `.error` means the project should not build; a | ||
| /// `.warning` is advisory. Throws only when the project can't be read at all | ||
| /// (missing directory / undecodable build-info), which is itself a failure. | ||
| public func lint(project: URL, requestedFormat: BuildInfoFormat?) throws -> [LintFinding] { | ||
| guard fileManager.directoryExists(at: project) else { | ||
| throw MunkiPkgError.message("\(project.path): Project not found.") | ||
| } | ||
| var findings: [LintFinding] = [] | ||
|
|
||
| // Decoding failures throw MunkiPkgError; surface them as a lint error | ||
| // rather than a crash so `--lint` always produces a report. | ||
| let configuration: PackageConfiguration | ||
| do { | ||
| configuration = try BuildInfoStore.load(from: project, requestedFormat: requestedFormat) | ||
| } catch let error as MunkiPkgError { | ||
| return [LintFinding(.error, error.description)] | ||
| } | ||
|
|
||
| if configuration.identifier.isEmpty { | ||
| findings.append(LintFinding(.error, "identifier is empty")) | ||
| } else if !Self.isReverseDNS(configuration.identifier) { | ||
| findings.append(LintFinding(.warning, "identifier \"\(configuration.identifier)\" is not reverse-DNS style")) | ||
| } | ||
|
|
||
| if configuration.version.isEmpty { | ||
| findings.append(LintFinding(.error, "version is empty")) | ||
| } | ||
|
|
||
| // A missing .pkg extension is not flagged: the build normalizes the | ||
| // resolved name to end in .pkg (matching munki-pkg), so it is harmless. | ||
| if configuration.name.isEmpty || configuration.name.contains("/") || configuration.name == "." || configuration.name == ".." { | ||
| findings.append(LintFinding(.error, "name \"\(configuration.name)\" must be a single path component")) | ||
| } | ||
|
|
||
| if configuration.notarization != nil, configuration.signing == nil { | ||
| findings.append(LintFinding(.warning, "notarization is configured but signing is not; notarization requires a Developer ID signature")) | ||
| } | ||
|
|
||
| let payload = project.appendingPathComponent("payload", isDirectory: true) | ||
| let scripts = project.appendingPathComponent("scripts", isDirectory: true) | ||
| let hasPayload = fileManager.directoryExists(at: payload) | ||
| let hasScripts = fileManager.directoryExists(at: scripts) | ||
| && ((try? fileManager.contents(at: scripts).contains { $0 != ".DS_Store" }) ?? false) | ||
| if !hasPayload, !hasScripts { | ||
| findings.append(LintFinding(.error, "project has neither a payload directory nor a non-empty scripts directory")) | ||
| } | ||
|
|
||
| if hasScripts { | ||
| findings.append(contentsOf: lintScripts(in: scripts)) | ||
| } | ||
|
|
||
| return findings | ||
| } | ||
|
|
||
| /// Reverse-DNS means at least two dot-separated, non-empty components, so | ||
| /// leading, repeated, and trailing dots (`.a`, `a..b`, `a.b.`) are rejected. | ||
| static func isReverseDNS(_ identifier: String) -> Bool { | ||
| let components = identifier.split(separator: ".", omittingEmptySubsequences: false) | ||
| return components.count >= 2 && components.allSatisfy { !$0.isEmpty } | ||
| } | ||
|
|
||
| private func lintScripts(in scripts: URL) -> [LintFinding] { | ||
| var findings: [LintFinding] = [] | ||
| for name in ["preinstall", "postinstall"] { | ||
| let script = scripts.appendingPathComponent(name) | ||
| var isDirectory: ObjCBool = false | ||
| guard fileManager.fileExists(atPath: script.path, isDirectory: &isDirectory) else { continue } | ||
| if isDirectory.boolValue { | ||
| findings.append(LintFinding(.error, "\(name) is a directory, but an install script must be a regular file")) | ||
| continue | ||
| } | ||
| if let data = fileManager.contents(atPath: script.path), !data.starts(with: Data("#!".utf8)) { | ||
| findings.append(LintFinding(.warning, "\(name) script does not start with a shebang (#!)")) | ||
| } | ||
| let permissions = (try? fileManager.attributesOfItem(atPath: script.path)[.posixPermissions] as? NSNumber)?.uint16Value ?? 0 | ||
| if permissions & 0o111 == 0 { | ||
| findings.append(LintFinding(.warning, "\(name) script is not executable")) | ||
| } | ||
| } | ||
| return findings | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| import Foundation | ||
| import Testing | ||
| @testable import SwiftPkgCore | ||
|
|
||
| struct LinterTests { | ||
|
|
||
| private func makeProject(buildInfo: String, addPayload: Bool = true) throws -> (TemporaryDirectory, URL) { | ||
| let temp = try TemporaryDirectory() | ||
| let project = temp.url.appendingPathComponent("P", isDirectory: true) | ||
| try FileManager.default.createDirectory(at: project, withIntermediateDirectories: false) | ||
| if addPayload { | ||
| let payload = project.appendingPathComponent("payload", isDirectory: true) | ||
| try FileManager.default.createDirectory(at: payload, withIntermediateDirectories: false) | ||
| try write("x", to: payload.appendingPathComponent("file.txt")) | ||
| } | ||
| try write(buildInfo, to: project.appendingPathComponent("build-info.json")) | ||
| return (temp, project) | ||
| } | ||
|
|
||
| @Test("a well-formed project produces no findings") | ||
| func cleanProject() throws { | ||
| let (temp, project) = try makeProject(buildInfo: #"{"name":"App-1.0.pkg","identifier":"com.example.app","version":"1.0"}"#) | ||
| defer { temp.remove() } | ||
| let findings = try Linter().lint(project: project, requestedFormat: nil) | ||
| #expect(findings.isEmpty) | ||
| } | ||
|
|
||
| @Test("errors on empty version and a traversal name") | ||
| func errorsOnBadVersionAndName() throws { | ||
| let (temp, project) = try makeProject(buildInfo: #"{"name":"../evil.pkg","identifier":"com.example.app","version":""}"#) | ||
| defer { temp.remove() } | ||
| let findings = try Linter().lint(project: project, requestedFormat: nil) | ||
| let errors = findings.filter { $0.severity == .error } | ||
| #expect(errors.contains { $0.message.contains("version is empty") }) | ||
| #expect(errors.contains { $0.message.contains("single path component") }) | ||
| } | ||
|
|
||
| @Test("warns on non-reverse-DNS identifier (a non-.pkg name is auto-normalized, not flagged)") | ||
| func warnsOnStyleIssues() throws { | ||
| let (temp, project) = try makeProject(buildInfo: #"{"name":"App","identifier":"noreverse","version":"1.0"}"#) | ||
| defer { temp.remove() } | ||
| let findings = try Linter().lint(project: project, requestedFormat: nil) | ||
| #expect(findings.allSatisfy { $0.severity == .warning }) | ||
| #expect(findings.contains { $0.message.contains("reverse-DNS") }) | ||
| #expect(!findings.contains { $0.message.contains(".pkg") }) | ||
| } | ||
|
|
||
| @Test("flags malformed dotted identifiers as non-reverse-DNS", arguments: [ | ||
| "noreverse", ".example", "com..example", "com.example.", | ||
| ]) | ||
| func warnsOnMalformedIdentifier(_ identifier: String) throws { | ||
| let (temp, project) = try makeProject(buildInfo: #"{"name":"App-1.0.pkg","identifier":"\#(identifier)","version":"1.0"}"#) | ||
| defer { temp.remove() } | ||
| let findings = try Linter().lint(project: project, requestedFormat: nil) | ||
| #expect(findings.contains { $0.message.contains("reverse-DNS") }) | ||
| } | ||
|
|
||
| @Test("accepts a well-formed reverse-DNS identifier") | ||
| func acceptsReverseDNS() throws { | ||
| let (temp, project) = try makeProject(buildInfo: #"{"name":"App-1.0.pkg","identifier":"com.example.app","version":"1.0"}"#) | ||
| defer { temp.remove() } | ||
| let findings = try Linter().lint(project: project, requestedFormat: nil) | ||
| #expect(!findings.contains { $0.message.contains("reverse-DNS") }) | ||
| } | ||
|
|
||
| @Test("errors when an install script is a directory") | ||
| func errorsOnScriptDirectory() throws { | ||
| let (temp, project) = try makeProject(buildInfo: #"{"name":"App-1.0.pkg","identifier":"com.example.app","version":"1.0"}"#) | ||
| defer { temp.remove() } | ||
| let scripts = project.appendingPathComponent("scripts", isDirectory: true) | ||
| let postinstall = scripts.appendingPathComponent("postinstall", isDirectory: true) | ||
| try FileManager.default.createDirectory(at: postinstall, withIntermediateDirectories: true) | ||
| let findings = try Linter().lint(project: project, requestedFormat: nil) | ||
| #expect(findings.contains { $0.severity == .error && $0.message.contains("is a directory") }) | ||
| } | ||
|
|
||
| @Test("a scripts-only project (no payload) lints cleanly") | ||
| func scriptsOnlyProjectIsClean() throws { | ||
| let (temp, project) = try makeProject(buildInfo: #"{"name":"App-1.0.pkg","identifier":"com.example.app","version":"1.0"}"#, addPayload: false) | ||
| defer { temp.remove() } | ||
| let scripts = project.appendingPathComponent("scripts", isDirectory: true) | ||
| try FileManager.default.createDirectory(at: scripts, withIntermediateDirectories: false) | ||
| let postinstall = scripts.appendingPathComponent("postinstall") | ||
| try write("#!/bin/sh\necho hi\n", to: postinstall) | ||
| try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: postinstall.path) | ||
| let findings = try Linter().lint(project: project, requestedFormat: nil) | ||
| #expect(findings.isEmpty) | ||
| } | ||
|
|
||
| @Test("warns when notarization is configured without signing") | ||
| func warnsNotarizationWithoutSigning() throws { | ||
| let buildInfo = #"{"name":"App-1.0.pkg","identifier":"com.example.app","version":"1.0","notarization_info":{"keychain_profile":"p"}}"# | ||
| let (temp, project) = try makeProject(buildInfo: buildInfo) | ||
| defer { temp.remove() } | ||
| let findings = try Linter().lint(project: project, requestedFormat: nil) | ||
| #expect(findings.contains { $0.message.contains("notarization is configured but signing") }) | ||
| } | ||
|
|
||
| @Test("errors when there is neither payload nor scripts") | ||
| func errorsOnEmptyProject() throws { | ||
| let (temp, project) = try makeProject(buildInfo: #"{"name":"App-1.0.pkg","identifier":"com.example.app","version":"1.0"}"#, addPayload: false) | ||
| defer { temp.remove() } | ||
| let findings = try Linter().lint(project: project, requestedFormat: nil) | ||
| #expect(findings.contains { $0.severity == .error && $0.message.contains("neither a payload") }) | ||
| } | ||
|
|
||
| /// Payload-free, since a scripts-only project is supported: the script | ||
| /// findings must be warnings, with no "neither a payload" error alongside. | ||
| @Test("warns on a non-executable script without a shebang") | ||
| func warnsOnBadScript() throws { | ||
| let (temp, project) = try makeProject(buildInfo: #"{"name":"App-1.0.pkg","identifier":"com.example.app","version":"1.0"}"#, addPayload: false) | ||
| defer { temp.remove() } | ||
| let scripts = project.appendingPathComponent("scripts", isDirectory: true) | ||
| try FileManager.default.createDirectory(at: scripts, withIntermediateDirectories: false) | ||
| let postinstall = scripts.appendingPathComponent("postinstall") | ||
| try write("echo hi\n", to: postinstall) // no shebang | ||
| try FileManager.default.setAttributes([.posixPermissions: 0o644], ofItemAtPath: postinstall.path) | ||
| let findings = try Linter().lint(project: project, requestedFormat: nil) | ||
| #expect(findings.contains { $0.message.contains("shebang") }) | ||
| #expect(findings.contains { $0.message.contains("not executable") }) | ||
| #expect(findings.allSatisfy { $0.severity == .warning }) | ||
| } | ||
|
|
||
| @Test("a scripts directory holding only .DS_Store does not count as scripts") | ||
| func emptyScriptsDirectoryStillErrors() throws { | ||
| let (temp, project) = try makeProject(buildInfo: #"{"name":"App-1.0.pkg","identifier":"com.example.app","version":"1.0"}"#, addPayload: false) | ||
| defer { temp.remove() } | ||
| let scripts = project.appendingPathComponent("scripts", isDirectory: true) | ||
| try FileManager.default.createDirectory(at: scripts, withIntermediateDirectories: false) | ||
| try write("", to: scripts.appendingPathComponent(".DS_Store")) | ||
| let findings = try Linter().lint(project: project, requestedFormat: nil) | ||
| #expect(findings.contains { $0.severity == .error && $0.message.contains("neither a payload") }) | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.