Skip to content

Commit 112363e

Browse files
kochj23claude
andcommitted
security: Fix command injection, CORS, force unwraps, add API auth
- CRITICAL: GitIntegrationTool rewritten to use Process.arguments array instead of bash -c shell interpolation (command injection) - Removed wildcard CORS from NovaAPIServer - Added bearer token auth on all POST endpoints - Fixed force unwrap in SessionManager (Application Support path) - Fixed force unwrap in LogViewerPanel (selectedCategories) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f07acf9 commit 112363e

4 files changed

Lines changed: 63 additions & 29 deletions

File tree

MLX Code/Services/NovaAPIServer.swift

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,17 @@ class NovaAPIServer {
3131
private var listener: NWListener?
3232
private let startTime = Date()
3333

34+
/// Local-only anti-CSRF bearer token (not a secret — just prevents drive-by POST from browser JS)
35+
private let apiToken: String = {
36+
let key = "NovaAPIToken"
37+
if let existing = UserDefaults.standard.string(forKey: key), !existing.isEmpty {
38+
return existing
39+
}
40+
let token = UUID().uuidString
41+
UserDefaults.standard.set(token, forKey: key)
42+
return token
43+
}()
44+
3445
private init() {}
3546

3647
func start() {
@@ -69,6 +80,13 @@ class NovaAPIServer {
6980
private func route(_ req: NovaRequest) async -> String {
7081
if req.method == "OPTIONS" { return http(200, "") }
7182

83+
// Require bearer token for all POST/DELETE requests (anti-CSRF)
84+
if req.method == "POST" || req.method == "DELETE" {
85+
guard let auth = req.headers["authorization"], auth == "Bearer \(apiToken)" else {
86+
return json(401, ["error": "Unauthorized — missing or invalid Bearer token"])
87+
}
88+
}
89+
7290
switch (req.method, req.path) {
7391

7492
case ("GET", "/api/status"):
@@ -185,6 +203,7 @@ class NovaAPIServer {
185203
let method: String
186204
let path: String
187205
let body: String
206+
let headers: [String: String]
188207
func bodyJSON() -> [String: Any]? {
189208
guard let data = body.data(using: .utf8) else { return nil }
190209
return try? JSONSerialization.jsonObject(with: data) as? [String: Any]
@@ -206,6 +225,7 @@ class NovaAPIServer {
206225
method = tokens[0]
207226
path = tokens[1].components(separatedBy: "?").first ?? tokens[1]
208227
body = rawBody
228+
headers = hdrs
209229
}
210230
}
211231

@@ -222,7 +242,7 @@ class NovaAPIServer {
222242
}
223243

224244
private func http(_ status: Int, _ body: String, _ ct: String = "text/plain") -> String {
225-
let st = [200:"OK",201:"Created",400:"Bad Request",404:"Not Found",500:"Internal Server Error"][status] ?? "Unknown"
226-
return "HTTP/1.1 \(status) \(st)\r\nContent-Type: \(ct); charset=utf-8\r\nContent-Length: \(body.utf8.count)\r\nAccess-Control-Allow-Origin: *\r\nConnection: close\r\n\r\n\(body)"
245+
let st = [200:"OK",201:"Created",400:"Bad Request",401:"Unauthorized",404:"Not Found",500:"Internal Server Error"][status] ?? "Unknown"
246+
return "HTTP/1.1 \(status) \(st)\r\nContent-Type: \(ct); charset=utf-8\r\nContent-Length: \(body.utf8.count)\r\nConnection: close\r\n\r\n\(body)"
227247
}
228248
}

MLX Code/Services/SessionManager.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ class SessionManager: ObservableObject {
1717

1818
private let fileManager = FileManager.default
1919
private var sessionFile: URL {
20-
let appSupport = fileManager.urls(for: .applicationSupportDirectory, in: .userDomainMask).first!
20+
guard let appSupport = fileManager.urls(for: .applicationSupportDirectory, in: .userDomainMask).first else {
21+
let fallback = fileManager.homeDirectoryForCurrentUser.appendingPathComponent(".mlxcode/Sessions")
22+
try? fileManager.createDirectory(at: fallback, withIntermediateDirectories: true)
23+
return fallback.appendingPathComponent("current_session.json")
24+
}
2125
let dir = appSupport.appendingPathComponent("MLX Code/Sessions")
2226
try? fileManager.createDirectory(at: dir, withIntermediateDirectories: true)
2327
return dir.appendingPathComponent("current_session.json")

MLX Code/Tools/GitIntegrationTool.swift

Lines changed: 35 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ class GitIntegrationTool: BaseTool {
9393
// MARK: - Git Operations
9494

9595
private func gitStatus(context: ToolContext) async throws -> ToolResult {
96-
let output = try await runGitCommand("status --porcelain -b", in: context.workingDirectory)
96+
let output = try await runGitCommand(["status", "--porcelain", "-b"], in: context.workingDirectory)
9797

9898
let lines = output.components(separatedBy: .newlines).filter { !$0.isEmpty }
9999
var branch = "unknown"
@@ -170,9 +170,13 @@ class GitIntegrationTool: BaseTool {
170170

171171
private func gitDiff(parameters: [String: Any], context: ToolContext) async throws -> ToolResult {
172172
let filePath = parameters["file_path"] as? String
173-
let command = filePath != nil ? "diff \(filePath!)" : "diff"
173+
var args = ["diff"]
174+
if let filePath = filePath {
175+
args.append("--")
176+
args.append(filePath)
177+
}
174178

175-
let output = try await runGitCommand(command, in: context.workingDirectory)
179+
let output = try await runGitCommand(args, in: context.workingDirectory)
176180

177181
var result = "# Git Diff\n\n"
178182

@@ -190,9 +194,10 @@ class GitIntegrationTool: BaseTool {
190194

191195
private func gitAdd(parameters: [String: Any], context: ToolContext) async throws -> ToolResult {
192196
let files = parameters["files"] as? [String] ?? ["."]
193-
let filesStr = files.joined(separator: " ")
197+
var args = ["add", "--"]
198+
args.append(contentsOf: files)
194199

195-
_ = try await runGitCommand("add \(filesStr)", in: context.workingDirectory)
200+
_ = try await runGitCommand(args, in: context.workingDirectory)
196201

197202
var result = "# Files Staged\n\n"
198203
for file in files {
@@ -208,16 +213,15 @@ class GitIntegrationTool: BaseTool {
208213

209214
if generateMessage || message == nil {
210215
// Generate commit message from diff
211-
let diff = try await runGitCommand("diff --cached", in: context.workingDirectory)
216+
let diff = try await runGitCommand(["diff", "--cached"], in: context.workingDirectory)
212217
message = generateCommitMessage(from: diff)
213218
}
214219

215220
guard let commitMessage = message, !commitMessage.isEmpty else {
216221
throw ToolError.missingParameter("Commit message is required")
217222
}
218223

219-
let escapedMessage = commitMessage.replacingOccurrences(of: "\"", with: "\\\"")
220-
_ = try await runGitCommand("commit -m \"\(escapedMessage)\"", in: context.workingDirectory)
224+
_ = try await runGitCommand(["commit", "-m", commitMessage], in: context.workingDirectory)
221225

222226
var result = "# Commit Created\n\n"
223227
result += "**Message**:\n```\n\(commitMessage)\n```\n\n"
@@ -227,7 +231,7 @@ class GitIntegrationTool: BaseTool {
227231
}
228232

229233
private func gitPush(context: ToolContext) async throws -> ToolResult {
230-
let output = try await runGitCommand("push", in: context.workingDirectory)
234+
let output = try await runGitCommand(["push"], in: context.workingDirectory)
231235

232236
var result = "# Push Complete\n\n"
233237
result += "```\n\(output)\n```\n"
@@ -236,7 +240,7 @@ class GitIntegrationTool: BaseTool {
236240
}
237241

238242
private func gitPull(context: ToolContext) async throws -> ToolResult {
239-
let output = try await runGitCommand("pull", in: context.workingDirectory)
243+
let output = try await runGitCommand(["pull"], in: context.workingDirectory)
240244

241245
var result = "# Pull Complete\n\n"
242246
result += "```\n\(output)\n```\n"
@@ -246,7 +250,7 @@ class GitIntegrationTool: BaseTool {
246250

247251
private func gitLog(parameters: [String: Any], context: ToolContext) async throws -> ToolResult {
248252
let limit = parameters["limit"] as? Int ?? 10
249-
let output = try await runGitCommand("log --oneline -n \(limit)", in: context.workingDirectory)
253+
let output = try await runGitCommand(["log", "--oneline", "-n", "\(limit)"], in: context.workingDirectory)
250254

251255
var result = "# Git Log (last \(limit) commits)\n\n"
252256
result += "```\n\(output)\n```\n"
@@ -257,8 +261,8 @@ class GitIntegrationTool: BaseTool {
257261
private func gitBranch(parameters: [String: Any], context: ToolContext) async throws -> ToolResult {
258262
let branchName = parameters["branch"] as? String
259263

260-
let command = branchName != nil ? "branch \(branchName!)" : "branch -a"
261-
let output = try await runGitCommand(command, in: context.workingDirectory)
264+
let args: [String] = branchName != nil ? ["branch", branchName!] : ["branch", "-a"]
265+
let output = try await runGitCommand(args, in: context.workingDirectory)
262266

263267
var result = "# Git Branches\n\n"
264268
result += "```\n\(output)\n```\n"
@@ -271,7 +275,7 @@ class GitIntegrationTool: BaseTool {
271275
throw ToolError.missingParameter("Branch name required for checkout")
272276
}
273277

274-
let output = try await runGitCommand("checkout \(branch)", in: context.workingDirectory)
278+
let output = try await runGitCommand(["checkout", branch], in: context.workingDirectory)
275279

276280
var result = "# Checked Out Branch\n\n"
277281
result += "**Branch**: \(branch)\n\n"
@@ -285,7 +289,7 @@ class GitIntegrationTool: BaseTool {
285289
throw ToolError.missingParameter("Branch name required for merge")
286290
}
287291

288-
let output = try await runGitCommand("merge \(branch)", in: context.workingDirectory)
292+
let output = try await runGitCommand(["merge", branch], in: context.workingDirectory)
289293

290294
var result = "# Merge Complete\n\n"
291295
result += "**Merged**: \(branch) into current branch\n\n"
@@ -299,7 +303,7 @@ class GitIntegrationTool: BaseTool {
299303
throw ToolError.missingParameter("file_path required for blame")
300304
}
301305

302-
let output = try await runGitCommand("blame \(filePath)", in: context.workingDirectory)
306+
let output = try await runGitCommand(["blame", "--", filePath], in: context.workingDirectory)
303307

304308
var result = "# Git Blame\n\n"
305309
result += "**File**: \(filePath)\n\n"
@@ -309,7 +313,7 @@ class GitIntegrationTool: BaseTool {
309313
}
310314

311315
private func gitStash(parameters: [String: Any], context: ToolContext) async throws -> ToolResult {
312-
let output = try await runGitCommand("stash", in: context.workingDirectory)
316+
let output = try await runGitCommand(["stash"], in: context.workingDirectory)
313317

314318
var result = "# Changes Stashed\n\n"
315319
result += "```\n\(output)\n```\n"
@@ -319,21 +323,27 @@ class GitIntegrationTool: BaseTool {
319323

320324
// MARK: - Helper Methods
321325

322-
private func runGitCommand(_ command: String, in directory: String) async throws -> String {
326+
/// Execute a git command safely using Process.arguments array form.
327+
/// SECURITY: Never passes arguments through a shell — prevents command injection.
328+
private func runGitCommand(_ args: [String], in directory: String) async throws -> String {
323329
let process = Process()
324-
process.executableURL = URL(fileURLWithPath: "/bin/bash")
330+
process.executableURL = URL(fileURLWithPath: "/usr/bin/git")
325331
process.currentDirectoryURL = URL(fileURLWithPath: directory)
326-
process.arguments = ["-c", "git \(command) 2>&1"]
332+
process.arguments = args
327333

328-
let pipe = Pipe()
329-
process.standardOutput = pipe
330-
process.standardError = pipe
334+
let stdoutPipe = Pipe()
335+
let stderrPipe = Pipe()
336+
process.standardOutput = stdoutPipe
337+
process.standardError = stderrPipe
331338

332339
try process.run()
333340
process.waitUntilExit()
334341

335-
let data = pipe.fileHandleForReading.readDataToEndOfFile()
336-
let output = String(data: data, encoding: .utf8) ?? ""
342+
let stdoutData = stdoutPipe.fileHandleForReading.readDataToEndOfFile()
343+
let stderrData = stderrPipe.fileHandleForReading.readDataToEndOfFile()
344+
let stdout = String(data: stdoutData, encoding: .utf8) ?? ""
345+
let stderr = String(data: stderrData, encoding: .utf8) ?? ""
346+
let output = stdout + (stderr.isEmpty ? "" : "\n" + stderr)
337347

338348
if process.terminationStatus != 0 && !output.isEmpty && output.contains("fatal") {
339349
throw ToolError.executionFailed("Git command failed: \(output)")

MLX Code/Views/LogViewerPanel.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ struct LogViewerPanel: View {
192192
if logManager.selectedCategories.isEmpty {
193193
return "All"
194194
} else if logManager.selectedCategories.count == 1 {
195-
return logManager.selectedCategories.first!
195+
return logManager.selectedCategories.first ?? "All"
196196
} else {
197197
return "\(logManager.selectedCategories.count) selected"
198198
}

0 commit comments

Comments
 (0)