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
34 changes: 34 additions & 0 deletions brain-bar/Sources/BrainBar/BrainDatabase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,40 @@ final class BrainDatabase: @unchecked Sendable {
}
}

func vacuumInto(targetPath: String) throws -> Int64 {
guard let db else { throw DBError.notOpen }
let trimmedTarget = targetPath.trimmingCharacters(in: .whitespacesAndNewlines)
guard !trimmedTarget.isEmpty else {
throw DBError.exec(SQLITE_MISUSE, "target_path is required")
}

let targetURL = URL(fileURLWithPath: trimmedTarget).standardizedFileURL
let sourceURL = URL(fileURLWithPath: path).standardizedFileURL
Comment on lines +479 to +480
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Medium BrainBar/BrainDatabase.swift:479

standardizedFileURL does not resolve symlinks, so the safety check at line 481 can be bypassed when paths differ by symlink. For example, on macOS /tmp is a symlink to /private/tmp, so /tmp/db.sqlite and /private/tmp/db.sqlite compare as different but refer to the same file, allowing VACUUM INTO to overwrite the live database. Consider using resolvingSymlinksInPath() before comparing.

-        let targetURL = URL(fileURLWithPath: trimmedTarget).standardizedFileURL
-        let sourceURL = URL(fileURLWithPath: path).standardizedFileURL
+        let targetURL = URL(fileURLWithPath: trimmedTarget).standardizedFileURL.resolvingSymlinksInPath()
+        let sourceURL = URL(fileURLWithPath: path).standardizedFileURL.resolvingSymlinksInPath()
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Sources/BrainBar/BrainDatabase.swift around lines 479-480:

`standardizedFileURL` does not resolve symlinks, so the safety check at line 481 can be bypassed when paths differ by symlink. For example, on macOS `/tmp` is a symlink to `/private/tmp`, so `/tmp/db.sqlite` and `/private/tmp/db.sqlite` compare as different but refer to the same file, allowing `VACUUM INTO` to overwrite the live database. Consider using `resolvingSymlinksInPath()` before comparing.

Evidence trail:
brain-bar/Sources/BrainBar/BrainDatabase.swift lines 472-498 (viewed at REVIEWED_COMMIT): `vacuumInto` uses `standardizedFileURL` at lines 479-480, compares paths at line 481. Apple documentation at https://developer.apple.com/documentation/foundation/url/standardizedfileurl confirms standardizedFileURL does not resolve symlinks. Apple documentation at https://developer.apple.com/documentation/foundation/nsurl/resolvingsymlinksinpath confirms resolvingSymlinksInPath does resolve symlinks. macOS `/tmp` → `/private/tmp` is a well-known symlink example.

guard targetURL.path != sourceURL.path else {
throw DBError.exec(SQLITE_MISUSE, "refusing to VACUUM INTO the live database path")
}

try FileManager.default.createDirectory(
at: targetURL.deletingLastPathComponent(),
withIntermediateDirectories: true
)
guard !FileManager.default.fileExists(atPath: targetURL.path) else {
throw DBError.exec(SQLITE_CANTOPEN, "backup target already exists: \(targetURL.path)")
}

var stmt: OpaquePointer?
let prepareRC = sqlite3_prepare_v2(db, "VACUUM INTO ?", -1, &stmt, nil)
guard prepareRC == SQLITE_OK, let stmt else { throw DBError.prepare(prepareRC) }
defer { sqlite3_finalize(stmt) }

bindText(targetURL.path, to: stmt, index: 1)
let stepRC = sqlite3_step(stmt)
guard stepRC == SQLITE_DONE else { throw DBError.step(stepRC) }

let attributes = try FileManager.default.attributesOfItem(atPath: targetURL.path)
return (attributes[.size] as? NSNumber)?.int64Value ?? 0
}

private static let allowedPragmas: Set<String> = [
"journal_mode", "busy_timeout", "cache_size", "synchronous",
"wal_checkpoint", "page_count", "page_size", "freelist_count"
Expand Down
45 changes: 44 additions & 1 deletion brain-bar/Sources/BrainBar/MCPRouter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,18 @@ final class MCPRouter: @unchecked Sendable {
}
}

private struct BackupVacuumResult: Encodable {
let status: String
let targetPath: String
let bytes: Int64

enum CodingKeys: String, CodingKey {
case status
case targetPath = "target_path"
case bytes
}
}

private var database: BrainDatabase?
let entityCache = EntityCache()
private static let defaultStringMaxLength = 256
Expand All @@ -39,7 +51,8 @@ final class MCPRouter: @unchecked Sendable {
"reason": 1_024,
"session_id": 128,
"source": 32,
"tag": 128
"tag": 128,
"target_path": 4_096
]
private static let stringArrayLimits: [String: (maxItems: Int, itemMaxLength: Int)] = [
"chunk_ids": (maxItems: 500, itemMaxLength: 128),
Expand Down Expand Up @@ -208,6 +221,8 @@ final class MCPRouter: @unchecked Sendable {
return try handleBrainAck(arguments)
case "brain_maintenance_rebuild_trigram":
return try handleBrainMaintenanceRebuildTrigram(arguments)
case "brain_backup_vacuum_into":
return try handleBrainBackupVacuumInto(arguments)
default:
throw ToolError.unknownTool(name)
}
Expand Down Expand Up @@ -625,6 +640,22 @@ final class MCPRouter: @unchecked Sendable {
)
}

private func handleBrainBackupVacuumInto(_ args: [String: Any]) throws -> ToolOutput {
guard let targetPath = args["target_path"] as? String else {
throw ToolError.missingParameter("target_path")
}
guard let db = database else { throw ToolError.noDatabase }
let bytes = try db.vacuumInto(targetPath: targetPath)
let payload = BackupVacuumResult(status: "ok", targetPath: targetPath, bytes: bytes)
return ToolOutput(
text: jsonEncode(payload),
metadata: [
"target_path": targetPath,
"bytes": bytes,
]
)
}

/// Safe JSON encoding — never use string interpolation with user data.
private func jsonEncode<T: Encodable>(_ value: T) -> String {
guard let data = try? JSONEncoder().encode(value),
Expand Down Expand Up @@ -1023,6 +1054,18 @@ final class MCPRouter: @unchecked Sendable {
"required": ["agent_id", "seq"]
] as [String: Any])
],
[
"name": "brain_backup_vacuum_into",
"description": "Create a SQLite backup snapshot using VACUUM INTO on BrainBar's single-writer connection.",
"annotations": MCPRouter.writeIdempotentAnnotations,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-idempotent backup tool annotated as idempotent

Low Severity

The brain_backup_vacuum_into tool uses writeIdempotentAnnotations which sets idempotentHint: true, but vacuumInto explicitly rejects repeat calls with the same target_path because it guards against the target file already existing. MCP clients seeing idempotentHint: true may automatically retry failed or timed-out calls, which will always fail on the second attempt with "backup target already exists."

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7034ddc. Configure here.

"inputSchema": MCPRouter.limitedInputSchema([
"type": "object",
"properties": [
"target_path": ["type": "string", "description": "Absolute path for the new SQLite backup file"],
] as [String: Any],
"required": ["target_path"]
] as [String: Any])
],
[
"name": "brain_maintenance_rebuild_trigram",
"description": "Operator-triggered maintenance command to rebuild the trigram FTS table in lock-aware batches.",
Expand Down
7 changes: 4 additions & 3 deletions brain-bar/Tests/BrainBarTests/MCPRouterTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,15 @@ final class MCPRouterTests: XCTestCase {
let tools = result?["tools"] as? [[String: Any]]

XCTAssertNotNil(tools)
XCTAssertEqual(tools?.count, 16, "Should have exactly 16 tools")
XCTAssertEqual(tools?.count, 17, "Should have exactly 17 tools")

let toolNames = Set(tools?.compactMap { $0["name"] as? String } ?? [])
let expected: Set<String> = [
"brain_search", "brain_store", "brain_recall", "brain_entity",
"brain_digest", "brain_update", "brain_expand", "brain_tags",
"brain_subscribe", "brain_unsubscribe", "brain_ack",
"brain_get_person", "brain_supersede", "brain_archive", "brain_enrich",
"brain_maintenance_rebuild_trigram",
"brain_backup_vacuum_into", "brain_maintenance_rebuild_trigram",
]
XCTAssertEqual(toolNames, expected)
}
Expand Down Expand Up @@ -168,7 +168,7 @@ final class MCPRouterTests: XCTestCase {

let tools = (response["result"] as? [String: Any])?["tools"] as? [[String: Any]] ?? []

XCTAssertEqual(tools.count, 16)
XCTAssertEqual(tools.count, 17)
for tool in tools {
XCTAssertNotNil(
tool["annotations"],
Expand Down Expand Up @@ -226,6 +226,7 @@ final class MCPRouterTests: XCTestCase {
"brain_subscribe": (false, false, false, false),
"brain_unsubscribe": (false, false, true, false),
"brain_ack": (false, false, true, false),
"brain_backup_vacuum_into": (false, false, true, false),
"brain_maintenance_rebuild_trigram": (false, false, true, false),
]

Expand Down
66 changes: 63 additions & 3 deletions brain-bar/Tests/BrainBarTests/SocketIntegrationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// Tests the full pipeline: connect to Unix socket → send Content-Length framed
// MCP request → receive Content-Length framed response.

import SQLite3
import XCTest
@testable import BrainBar

Expand Down Expand Up @@ -91,7 +92,7 @@ final class SocketIntegrationTests: XCTestCase {

let tools = (response["result"] as? [String: Any])?["tools"] as? [[String: Any]]
XCTAssertNotNil(tools)
XCTAssertEqual(tools?.count, 16)
XCTAssertEqual(tools?.count, 17)

let encodedResponse = try MCPFraming.encodeJSONResponse(response)
XCTAssertGreaterThan(encodedResponse.count, 8192)
Expand Down Expand Up @@ -135,7 +136,7 @@ final class SocketIntegrationTests: XCTestCase {
8192,
"Claude Desktop's MCPB utility process parses raw extension stdout in 8192-byte chunks"
)
XCTAssertEqual(tools?.count, 16)
XCTAssertEqual(tools?.count, 17)
for tool in tools ?? [] {
XCTAssertNil(
tool["annotations"],
Expand Down Expand Up @@ -168,6 +169,53 @@ final class SocketIntegrationTests: XCTestCase {
XCTAssertNotNil(response["result"])
}

func testBrainBackupVacuumIntoOverSocketCreatesRestorableSnapshot() throws {
let targetPath = NSTemporaryDirectory() + "brainbar-backup-\(UUID().uuidString).db"
defer { try? FileManager.default.removeItem(atPath: targetPath) }

_ = try sendMCPRequest([
"jsonrpc": "2.0", "id": 20, "method": "initialize",
"params": ["protocolVersion": "2024-11-05", "capabilities": [:] as [String: Any],
"clientInfo": ["name": "backup-test", "version": "1.0"]]
])
_ = try sendMCPRequest([
"jsonrpc": "2.0",
"id": 21,
"method": "tools/call",
"params": [
"name": "brain_store",
"arguments": [
"content": "vacuum over socket",
"tags": ["backup-test"]
] as [String: Any]
]
])

let response = try sendMCPRequest([
"jsonrpc": "2.0",
"id": 22,
"method": "tools/call",
"params": [
"name": "brain_backup_vacuum_into",
"arguments": ["target_path": targetPath]
]
])

XCTAssertNil(response["error"])
let result = response["result"] as? [String: Any]
XCTAssertEqual(result?["isError"] as? Bool, nil)
XCTAssertTrue(FileManager.default.fileExists(atPath: targetPath))

var restored: OpaquePointer?
XCTAssertEqual(sqlite3_open_v2(targetPath, &restored, SQLITE_OPEN_READONLY, nil), SQLITE_OK)
defer { sqlite3_close(restored) }
XCTAssertEqual(try queryString("PRAGMA integrity_check", on: restored), "ok")
XCTAssertEqual(
try queryString("SELECT content FROM chunks WHERE content = 'vacuum over socket'", on: restored),
"vacuum over socket"
)
}

func testMCPBrainSubscribeOverSocketReturnsCursorState() throws {
_ = try sendMCPRequest([
"jsonrpc": "2.0", "id": 1, "method": "initialize",
Expand Down Expand Up @@ -705,7 +753,7 @@ final class SocketIntegrationTests: XCTestCase {

let toolsResponse = try JSONSerialization.jsonObject(with: Data(outputLines[1].utf8)) as? [String: Any]
let tools = (toolsResponse?["result"] as? [String: Any])?["tools"] as? [[String: Any]]
XCTAssertEqual(tools?.count, 16)
XCTAssertEqual(tools?.count, 17)
}

// MARK: - C2: Socket path length validation
Expand Down Expand Up @@ -741,6 +789,18 @@ final class SocketIntegrationTests: XCTestCase {
return try readMCPMessage(fd: fd)
}

private func queryString(_ sql: String, on db: OpaquePointer?) throws -> String? {
var stmt: OpaquePointer?
let rc = sqlite3_prepare_v2(db, sql, -1, &stmt, nil)
guard rc == SQLITE_OK else {
throw NSError(domain: "sqlite", code: Int(rc), userInfo: [NSLocalizedDescriptionKey: "prepare failed \(rc)"])
}
defer { sqlite3_finalize(stmt) }
guard sqlite3_step(stmt) == SQLITE_ROW else { return nil }
guard let value = sqlite3_column_text(stmt, 0) else { return nil }
return String(cString: value)
}

private func connectClient() throws -> Int32 {
let fd = socket(AF_UNIX, SOCK_STREAM, 0)
guard fd >= 0 else { throw NSError(domain: "test", code: 1, userInfo: [NSLocalizedDescriptionKey: "socket() failed"]) }
Expand Down
2 changes: 2 additions & 0 deletions scripts/launchd/backup-daily.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ set -euo pipefail

export PATH="/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:$HOME/.local/bin"
export PYTHONUNBUFFERED=1
: "${BRAINLAYER_BACKUP_TIMEOUT_SECONDS:=300}"
export BRAINLAYER_BACKUP_TIMEOUT_SECONDS
BRAINLAYER_DIR="${BRAINLAYER_DIR:-__BRAINLAYER_DIR_VALUE__}"
case "$BRAINLAYER_DIR" in
__BRAINLAYER_DIR_*) BRAINLAYER_DIR="$HOME/Gits/brainlayer" ;;
Expand Down
3 changes: 3 additions & 0 deletions scripts/launchd/com.brainlayer.backup-daily.plist
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
<key>Nice</key>
<integer>15</integer>

<key>ExitTimeOut</key>
Comment on lines 39 to +40
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Enforce the backup timeout in the job itself

For runs that hang while the process is active, such as during Drive upload after the snapshot is made, this key does not impose a 5-minute runtime limit. I checked the launchd.plist(5) semantics: ExitTimeOut is the grace period between launchd sending SIGTERM and then SIGKILL when it is already stopping a job, not a maximum wall-clock duration. Since this agent only uses StartCalendarInterval and the wrapper has no whole-job watchdog, a stuck backup can still remain alive indefinitely; the timeout needs to be enforced in backup-daily.sh or backup_daily.py if the daily job must be bounded.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed. The launchd wrapper now exports BRAINLAYER_BACKUP_TIMEOUT_SECONDS=300 by default, and backup_daily.main() enforces that wall-clock timeout with SIGALRM and exits 124 on timeout. Added a regression test for the timeout path.

<integer>300</integer>

<key>ProcessType</key>
<string>Background</string>
</dict>
Expand Down
Loading
Loading