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
8 changes: 8 additions & 0 deletions Sources/mcs/Core/Environment.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ struct Environment {
mcsDirectory.appendingPathComponent(Constants.ExternalPacks.packsDirectory)
}

/// Whether the `MCS_DEBUG` env var is set. Used to gate developer-facing diagnostic
/// emits (e.g. stderr writes for non-fatal failures that would otherwise be silent).
/// Production runs are unaffected; CI and developer shells can opt in by exporting
/// `MCS_DEBUG=1`.
static var isDebugMode: Bool {
ProcessInfo.processInfo.environment["MCS_DEBUG"] != nil
}

/// YAML registry of installed external packs (`~/.mcs/registry.yaml`).
var packsRegistry: URL {
mcsDirectory.appendingPathComponent(Constants.ExternalPacks.registryFilename)
Expand Down
281 changes: 254 additions & 27 deletions Sources/mcs/Core/UpdateChecker.swift

Large diffs are not rendered by default.

18 changes: 14 additions & 4 deletions Sources/mcs/ExternalPack/PackFetcher.swift
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,7 @@ struct PackFetcher {

/// Validate that a git ref is safe for use as a command argument.
func validateRef(_ ref: String) throws {
guard !ref.hasPrefix("-"),
!ref.contains(".."),
ref.range(of: #"^[a-zA-Z0-9._/+-]+$"#, options: .regularExpression) != nil
else {
guard isValidGitRef(ref) else {
throw PackFetchError.invalidRef(ref)
}
}
Expand All @@ -177,6 +174,19 @@ struct PackFetcher {
}
}

// MARK: - Ref validation

/// Predicate form of `PackFetcher.validateRef`. Returns `true` iff `ref` is safe to pass
/// directly as a positional argument to git: not a `-` option, no `..` range, and matches
/// a conservative subset of characters that git accepts (rejects e.g. `@`, which git allows
/// outside `@{}`). Used by callers that want to silently skip invalid refs — registry
/// corruption shouldn't surface as a git error.
func isValidGitRef(_ ref: String) -> Bool {
!ref.hasPrefix("-")
&& !ref.contains("..")
&& ref.range(of: #"^[a-zA-Z0-9._/+-]+$"#, options: .regularExpression) != nil
Comment thread
bguidolim marked this conversation as resolved.
}

// MARK: - Errors

/// Errors that can occur during pack fetch operations.
Expand Down
12 changes: 11 additions & 1 deletion Sources/mcs/ExternalPack/PackHeuristics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ enum PackHeuristics {
}

/// Directories at the pack root that are infrastructure, not pack content.
private static let ignoredDirectories: Set<String> = [
static let ignoredDirectories: Set<String> = [
".git", ".github", ".gitlab", ".vscode",
"node_modules", "__pycache__", ".build",
]
Expand Down Expand Up @@ -168,13 +168,23 @@ enum PackHeuristics {
}

/// Files at the pack root that are expected infrastructure, not content.
/// Used by `checkRootLevelContentFiles` to avoid warning about these files.
/// Intentionally includes `techpack.yaml` — see `infrastructureFilesForUpdateCheck`
/// below for the (deliberately different) set the update-check filter uses.
private static let infrastructureFiles: Set<String> = [
Constants.ExternalPacks.manifestFilename, "README.md", "README", "LICENSE", "LICENSE.md",
"CHANGELOG.md", "CONTRIBUTING.md", ".gitignore", ".editorconfig",
"package.json", "package-lock.json", "requirements.txt",
"Makefile", "Dockerfile", ".dockerignore",
]

/// Used by `UpdateChecker`'s noise filter. Excludes `techpack.yaml` because
/// manifest edits can swap the install surface (hooks, MCP commands) — silently
/// suppressing them would be a supply-chain attack vector. Do not deduplicate
/// these sets.
static let infrastructureFilesForUpdateCheck: Set<String> =
infrastructureFiles.subtracting([Constants.ExternalPacks.manifestFilename])

private static func checkRootLevelContentFiles(
manifest: ExternalPackManifest,
packPath: URL
Expand Down
32 changes: 22 additions & 10 deletions Sources/mcs/ExternalPack/PackRegistryFile.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,17 @@ struct PackRegistryFile {
let path: URL // ~/.mcs/registry.yaml

struct PackEntry: Codable, Equatable {
let identifier: String
let displayName: String
let author: String?
let sourceURL: String // Git clone URL or original local path
let ref: String? // Git tag/branch/commit
let commitSHA: String // Exact commit (git) or "local" (local packs)
let localPath: String // Relative to ~/.mcs/packs/ (git) or absolute path (local)
let addedAt: String // ISO 8601 date
let trustedScriptHashes: [String: String] // relativePath -> SHA-256
let isLocal: Bool? // nil/false = git pack, true = local filesystem pack
// `var` only to enable copy-and-mutate via `withCommitSHA`; treat as immutable elsewhere.
var identifier: String
var displayName: String
var author: String?
var sourceURL: String // Git clone URL or original local path
var ref: String? // Git tag/branch/commit
var commitSHA: String // Exact commit (git) or "local" (local packs)
var localPath: String // Relative to ~/.mcs/packs/ (git) or absolute path (local)
var addedAt: String // ISO 8601 date
var trustedScriptHashes: [String: String] // relativePath -> SHA-256
var isLocal: Bool? // nil/false = git pack, true = local filesystem pack

/// Whether this pack is a local filesystem pack (not cloned via git).
var isLocalPack: Bool {
Expand All @@ -31,6 +32,17 @@ struct PackRegistryFile {
}
return PathContainment.safePath(relativePath: localPath, within: packsDirectory)
}

/// Return a copy with `commitSHA` replaced. The registry SHA and working-tree
/// HEAD are normally kept in lockstep (set together by clone/checkout). Updating
/// only the SHA reflects "we acknowledged this upstream commit but did not move
/// the checkout" — used when a non-material upstream commit is recognized but
/// not pulled.
func withCommitSHA(_ sha: String) -> Self {
var copy = self
copy.commitSHA = sha
return copy
}
}

struct RegistryData: Codable {
Expand Down
28 changes: 28 additions & 0 deletions Tests/MCSTests/PackHeuristicsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -684,4 +684,32 @@ struct PackHeuristicsTests {
#expect(!dirWarnings.isEmpty)
#expect(dirWarnings.allSatisfy { $0.severity == .warning })
}

// MARK: - infrastructureFilesForUpdateCheck (issue #338)

@Test("infrastructureFilesForUpdateCheck excludes techpack.yaml (supply-chain invariant)")
func updateCheckSetExcludesManifest() {
#expect(!PackHeuristics.infrastructureFilesForUpdateCheck.contains(
Constants.ExternalPacks.manifestFilename
))
}

@Test("infrastructureFilesForUpdateCheck still contains the other infra files")
func updateCheckSetContainsInfraFiles() {
let set = PackHeuristics.infrastructureFilesForUpdateCheck
#expect(set.contains("README.md"))
#expect(set.contains("LICENSE"))
#expect(set.contains("CHANGELOG.md"))
#expect(set.contains(".gitignore"))
#expect(set.contains("Makefile"))
}

@Test("ignoredDirectories is accessible and contains expected entries")
func ignoredDirsAccessible() {
let dirs = PackHeuristics.ignoredDirectories
#expect(dirs.contains(".git"))
#expect(dirs.contains(".github"))
#expect(dirs.contains("node_modules"))
#expect(dirs.contains(".build"))
}
}
35 changes: 32 additions & 3 deletions Tests/MCSTests/TestHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,28 @@ final class MockShellRunner: ShellRunning, @unchecked Sendable {

let environment: Environment

/// Mock is `@unchecked Sendable` and may be called concurrently from
/// `DispatchQueue.concurrentPerform`; the lock makes the queue and call arrays consistent.
private let lock = NSLock()

var runCalls: [RunCall] = []
var shellCalls: [ShellCall] = []
var commandExistsCalls: [String] = []

/// Result returned from `run()` and `shell()`. Defaults to success.
/// Default result when neither queue below produces a value.
/// Dispatch precedence in `run()`: `runResultsByFirstArg[arguments.first]` →
/// `runResults.removeFirst()` → `result`.
var result = ShellResult(exitCode: 0, stdout: "", stderr: "")

/// Sequential results for `run()`: pops first element when non-empty, falls back to `result`.
/// Positional queue for `run()`. Only safe for sequential call orders.
var runResults: [ShellResult] = []

/// Sequential results for `shell()`: pops first element when non-empty, falls back to `result`.
/// Argument-keyed dispatch for `run()`, keyed on `arguments.first`. Use this for tests
/// where `concurrentPerform` interleaves calls — every matching call returns the same
/// canned response regardless of order.
var runResultsByFirstArg: [String: ShellResult] = [:]

/// Positional queue for `shell()`. Same precedence model as `runResults`.
var shellResults: [ShellResult] = []

/// Controls what `commandExists()` returns. Defaults to `true`.
Expand All @@ -102,6 +113,8 @@ final class MockShellRunner: ShellRunning, @unchecked Sendable {
}

func commandExists(_ command: String) -> Bool {
lock.lock()
defer { lock.unlock() }
commandExistsCalls.append(command)
return commandExistsResult
}
Expand All @@ -114,13 +127,18 @@ final class MockShellRunner: ShellRunning, @unchecked Sendable {
additionalEnvironment: [String: String],
interactive: Bool
) -> ShellResult {
lock.lock()
defer { lock.unlock() }
runCalls.append(RunCall(
executable: executable,
arguments: arguments,
workingDirectory: workingDirectory,
additionalEnvironment: additionalEnvironment,
interactive: interactive
))
if let firstArg = arguments.first, let scripted = runResultsByFirstArg[firstArg] {
return scripted
}
if !runResults.isEmpty {
return runResults.removeFirst()
}
Expand All @@ -134,6 +152,8 @@ final class MockShellRunner: ShellRunning, @unchecked Sendable {
additionalEnvironment: [String: String],
interactive: Bool
) -> ShellResult {
lock.lock()
defer { lock.unlock() }
shellCalls.append(ShellCall(
command: command,
workingDirectory: workingDirectory,
Expand Down Expand Up @@ -305,6 +325,15 @@ func makeLocalRegistryEntry(
)
}

/// Create the on-disk pack clone directory under `Environment(home:).packsDirectory` so
/// `PackEntry.resolvedPath(packsDirectory:)` resolves and `PathContainment.safePath` allows
/// it. The directory is empty — tests that mock the shell don't need a real git checkout,
/// only a valid working directory to pass to git invocations.
func preparePackDir(home: URL, identifier: String) throws {
let dir = Environment(home: home).packsDirectory.appendingPathComponent(identifier)
try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true)
}

// MARK: - Temp Directory Helpers

/// Create a bare temp directory with a UUID-unique name.
Expand Down
Loading
Loading