Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Sources/mcs/Commands/PackCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -678,10 +678,11 @@ struct UpdatePack: LockedCommand {
switch result {
case .alreadyUpToDate:
ctx.output.success("\(entry.displayName): already up to date")
case let .updated(updatedEntry):
case let .updated(updatedEntry, diff):
ctx.registry.register(updatedEntry, in: &updatedData)
updatedCount += 1
ctx.output.success("\(entry.displayName): updated (\(updatedEntry.shortSHA))")
ctx.output.packChangeSummary(diff)
case .trustDeclined:
ctx.output.info("\(entry.identifier): \(result.reason ?? "trust not granted") (will re-prompt next run)")
case .fetchFailed, .manifestInvalid, .internalError:
Expand Down
3 changes: 2 additions & 1 deletion Sources/mcs/Commands/UpdateCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,11 @@ struct UpdateCommand: LockedCommand {
switch result {
case .alreadyUpToDate:
output.dimmed(" \(entry.displayName): already up to date")
case let .updated(updatedEntry):
case let .updated(updatedEntry, diff):
registryFile.register(updatedEntry, in: &updatedData)
anyUpdated = true
output.success(" \(entry.displayName): \(entry.shortSHA) → \(updatedEntry.shortSHA)")
output.packChangeSummary(diff, indent: " ")
case .trustDeclined:
output.info(" \(entry.identifier): \(result.reason ?? "trust not granted") (will re-prompt on next 'mcs update')")
skipped.insert(entry.identifier)
Expand Down
17 changes: 11 additions & 6 deletions Sources/mcs/Core/GitignoreManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,24 @@ struct GitignoreManager {

/// Resolve the global gitignore file path.
/// Checks `git config core.excludesFile`, falls back to `~/.config/git/ignore`.
///
/// Every path here is anchored to `shell.environment.homeDirectory`, never to the process's
/// own home. In production the two are the same (`Environment()` derives from
/// `NSHomeDirectory()`), but they differ under an injected `Environment(home:)` — and this
/// type both reads *and writes* the file, so resolving the process home let a sandboxed
/// caller mutate the real user's global gitignore. `HOME` is passed to `git config` for the
/// same reason: `--global` resolves `$HOME/.gitconfig`, which would otherwise escape too.
func resolveGlobalGitignorePath() -> URL {
let home = shell.environment.homeDirectory
let result = shell.run(
shell.environment.gitPath,
arguments: ["config", "--global", "core.excludesFile"]
arguments: ["config", "--global", "core.excludesFile"],
additionalEnvironment: ["HOME": home.path]
)
if result.succeeded, !result.stdout.isEmpty {
let path = result.stdout.replacingOccurrences(
of: "~",
with: FileManager.default.homeDirectoryForCurrentUser.path
)
let path = result.stdout.replacingOccurrences(of: "~", with: home.path)
return URL(fileURLWithPath: path)
}
let home = FileManager.default.homeDirectoryForCurrentUser
return home
.appendingPathComponent(".config")
.appendingPathComponent("git")
Expand Down
94 changes: 85 additions & 9 deletions Sources/mcs/ExternalPack/ExternalPackManifest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ extension ExternalPackManifest {

private func validateIgnoreEntries() throws {
guard let ignore, !ignore.isEmpty else { return }
let referenced = PackHeuristics.referencedPaths(from: self)
let referenced = referencedPaths
for entry in ignore {
if let rejection = Self.classifyIgnoreEntry(entry, referenced: referenced) {
throw ManifestError.ignoreEntryLoadBearing(entry: entry, reason: rejection.reason)
Expand Down Expand Up @@ -304,7 +304,7 @@ extension ExternalPackManifest {
rejected: (_ entry: String, _ rejection: IgnoreEntryRejection) -> Void
) -> ExternalPackManifest {
guard let ignore, !ignore.isEmpty else { return self }
let referenced = PackHeuristics.referencedPaths(from: self)
let referenced = referencedPaths
var kept: [String] = []
for entry in ignore {
if let rejection = Self.classifyIgnoreEntry(entry, referenced: referenced) {
Expand Down Expand Up @@ -462,7 +462,7 @@ enum ManifestError: Error, Equatable, LocalizedError {
///
/// Shorthand keys: `brew`, `mcp`, `plugin`, `shell`, `hook`, `command`,
/// `skill`, `settingsFile`, `gitignore`. See `ShorthandKeys` for details.
struct ExternalComponentDefinition: Codable {
struct ExternalComponentDefinition: Codable, Equatable {
var id: String
let displayName: String
let description: String
Expand Down Expand Up @@ -734,7 +734,7 @@ enum ExternalInstallActionType: String, Codable {
}

/// Declarative install action types that can be expressed in YAML.
enum ExternalInstallAction: Codable {
enum ExternalInstallAction: Codable, Equatable {
case mcpServer(ExternalMCPServerConfig)
case plugin(name: String)
case brewInstall(package: String)
Expand Down Expand Up @@ -830,7 +830,7 @@ enum ExternalInstallAction: Codable {
// MARK: - MCP Server Config

/// Configuration for an MCP server declared in an external pack manifest.
struct ExternalMCPServerConfig: Codable {
struct ExternalMCPServerConfig: Codable, Equatable {
let name: String
let command: String?
let args: [String]?
Expand Down Expand Up @@ -868,7 +868,7 @@ enum ExternalScope: String, Codable {
// MARK: - Copy Pack File Config

/// Configuration for copying a file from the pack into the Claude directory.
struct ExternalCopyPackFileConfig: Codable {
struct ExternalCopyPackFileConfig: Codable, Equatable {
let source: String
let destination: String
let fileType: ExternalCopyFileType?
Expand All @@ -882,10 +882,86 @@ enum ExternalCopyFileType: String, Codable {
case generic
}

// MARK: - Referenced Pack Files

/// Normalize a pack-relative path so equivalent expressions collapse to the same key.
/// Trims whitespace and strips a leading `./` so `hooks/foo.sh`, `./hooks/foo.sh`, and
/// ` hooks/foo.sh` are one path.
///
/// Private so the only way to obtain a pack-relative path is through the `referencedSource` of
/// the declaration that owns it — `ignore:` validation, unreferenced-file linting, and
/// `PackDiff`'s content hashing all key on these strings, and a consumer that normalized
/// differently would silently drop a file from one side of a comparison.
private func normalizedPackPath(_ path: String) -> String {
var normalized = path.trimmingCharacters(in: .whitespacesAndNewlines)
if normalized.hasPrefix("./") { normalized = String(normalized.dropFirst(2)) }
return normalized
}

extension ExternalComponentDefinition {
/// The pack-relative file this component installs, normalized, or `nil` when the
/// component's install action copies nothing (brew, MCP, plugin, shell, gitignore).
var referencedSource: String? {
switch installAction {
case let .copyPackFile(config):
normalizedPackPath(config.source)
case let .settingsFile(source):
normalizedPackPath(source)
case .brewInstall, .gitignoreEntries, .mcpServer, .plugin, .settingsMerge, .shellCommand:
nil
}
}

/// Whether `source` copies the pack root wholesale (`.`), which would sweep in
/// `techpack.yaml`, `LICENSE`, and `README`.
var copiesPackRoot: Bool {
guard case let .copyPackFile(config) = installAction else { return false }
let normalized = normalizedPackPath(config.source)
return normalized.isEmpty || normalized == "."
}
}

extension ExternalTemplateDefinition {
/// The pack-relative content file backing this template section, normalized.
var referencedSource: String {
normalizedPackPath(contentFile)
}
}

extension ExternalConfigureProject {
/// The pack-relative configure script, normalized.
var referencedSource: String {
normalizedPackPath(script)
}
}

extension ExternalPackManifest {
/// Every pack-relative path this manifest declares, normalized.
///
/// Lives on the manifest because it is a property of what the pack declares, not a quality
/// heuristic: `validate()` rejects `ignore:` entries that would mask one of these,
/// `PackHeuristics` reports the files that are *not* here, and `PackSnapshot` hashes them.
var referencedPaths: Set<String> {
var paths = Set<String>()
for component in components ?? [] {
if let source = component.referencedSource {
paths.insert(source)
}
}
for template in templates ?? [] {
paths.insert(template.referencedSource)
}
if let configureProject {
paths.insert(configureProject.referencedSource)
}
return paths
}
}

// MARK: - Templates

/// A template contribution declared in an external pack manifest.
struct ExternalTemplateDefinition: Codable {
struct ExternalTemplateDefinition: Codable, Equatable {
var sectionIdentifier: String
let placeholders: [String]?
let contentFile: String
Expand All @@ -895,14 +971,14 @@ struct ExternalTemplateDefinition: Codable {
// MARK: - Configure Project

/// Script-based project configuration hook.
struct ExternalConfigureProject: Codable {
struct ExternalConfigureProject: Codable, Equatable {
let script: String
}

// MARK: - Doctor Checks

/// A declarative doctor check definition for external packs.
struct ExternalDoctorCheckDefinition: Codable {
struct ExternalDoctorCheckDefinition: Codable, Equatable {
let type: ExternalDoctorCheckType
let name: String
let section: String?
Expand Down
54 changes: 6 additions & 48 deletions Sources/mcs/ExternalPack/PackHeuristics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,10 @@ enum PackHeuristics {
components: [ExternalComponentDefinition]
) -> [Finding] {
var findings: [Finding] = []
for component in components {
if case let .copyPackFile(config) = component.installAction {
var normalized = config.source.trimmingCharacters(in: .whitespaces)
if normalized.hasPrefix("./") {
normalized = String(normalized.dropFirst(2))
}
if normalized.isEmpty || normalized == "." {
let msg = "Component '\(component.id)' uses source '.' which copies the entire pack root"
+ " (including techpack.yaml, LICENSE, README)"
findings.append(Finding(severity: .error, message: msg))
}
}
for component in components where component.copiesPackRoot {
let msg = "Component '\(component.id)' uses source '.' which copies the entire pack root"
+ " (including techpack.yaml, LICENSE, README)"
findings.append(Finding(severity: .error, message: msg))
}
return findings
}
Expand Down Expand Up @@ -100,46 +92,12 @@ enum PackHeuristics {
"node_modules", "__pycache__", ".build",
]

/// Paths the manifest relies on for its install surface.
/// Used by `checkUnreferencedFiles`, by `ExternalPackManifest.validate()` to reject
/// load-bearing entries in the `ignore:` list (issue #338), and by the runtime safety
/// guard in `ExternalPackLoader` that strips forbidden `ignore:` entries defensively.
static func referencedPaths(from manifest: ExternalPackManifest) -> Set<String> {
var paths = Set<String>()
for component in manifest.components ?? [] {
switch component.installAction {
case let .copyPackFile(config):
paths.insert(normalizeReferencedPath(config.source))
case let .settingsFile(source):
paths.insert(normalizeReferencedPath(source))
default:
break
}
}
for template in manifest.templates ?? [] {
paths.insert(normalizeReferencedPath(template.contentFile))
}
if let script = manifest.configureProject?.script {
paths.insert(normalizeReferencedPath(script))
}
return paths
}

/// Normalize a referenced path so equivalent expressions collapse to the same key.
/// Trims whitespace and strips a leading `./` so `ignore:` validation doesn't miss
/// the same file expressed as `hooks/foo.sh` vs `./hooks/foo.sh` vs ` hooks/foo.sh`.
private static func normalizeReferencedPath(_ path: String) -> String {
var normalized = path.trimmingCharacters(in: .whitespacesAndNewlines)
if normalized.hasPrefix("./") { normalized = String(normalized.dropFirst(2)) }
return normalized
}

private static func checkUnreferencedFiles(
manifest: ExternalPackManifest,
packPath: URL
) -> [Finding] {
let fm = FileManager.default
let referencedPaths = referencedPaths(from: manifest)
let referencedPaths = manifest.referencedPaths

let rootContents: [URL]
do {
Expand Down Expand Up @@ -232,7 +190,7 @@ enum PackHeuristics {
packPath: URL
) -> [Finding] {
let fm = FileManager.default
let referencedRootFiles = referencedPaths(from: manifest).filter { !$0.contains("/") }
let referencedRootFiles = manifest.referencedPaths.filter { !$0.contains("/") }

let rootContents: [URL]
do {
Expand Down
Loading
Loading