Close the three holes the #1486 review found (#1488)

* Fix the three follow-ups from the #1486 review

Three defects shipped with the censorship-resilience merge, all
confirmed against main:

1. Source-manifest verification silently accepted added files.
   shasum -c checks only the files the manifest lists, and the Xcode
   project compiles every source file present in the tree — so a
   hostile mirror could pass verification by adding a file rather than
   modifying one. The manifest header and VERIFYING-A-BUILD.md now
   require the completeness check (git status --porcelain, or a path
   diff for tarballs) alongside the hash check.

2. A relay removed while Tor was bootstrapping reconnected anyway.
   dropRelays never subtracted from pendingTorConnectionURLs, and a
   custom relay passes the allow-list filter, so draining the pending
   queue resurrected a relay someone had explicitly deleted.

3. Turning Tor off mid-bootstrap read as 'network may be blocking tor'.
   shutdownCompletely left the detached 75s poll loop running, which
   then stamped bootstrapDidStall over the clean shutdown state; and
   the stall handler guarded on torEnforced, which is compile-time true
   in release, instead of the runtime preference. The poll loop is now
   generation-fenced (shutdown, dormancy, and restart each invalidate
   it) and the handler consults persistedTorPreference().

Both app-side fixes carry regression tests proven to fail pre-fix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address Codex review: ignored files and manifest placement

git status --porcelain omits ignored paths, and .gitignore covers
build/ — a planted bitchat/build/Evil.swift would compile via the
synchronized group while the documented check stayed silent. The
checkout check now uses --ignored.

The downloaded manifest also has to live outside the tree, or it trips
the completeness checks itself; the doc now says so and references it
at /tmp throughout.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: jack <jackjackbits@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
jack
2026-07-26 21:01:38 +02:00
committed by GitHub
co-authored by jack Claude Fable 5
parent c1ce9029d8
commit 132120a88e
7 changed files with 135 additions and 23 deletions
+7 -1
View File
@@ -63,7 +63,13 @@ jobs:
echo "#" echo "#"
echo "# Verify a checkout of this ref with:" echo "# Verify a checkout of this ref with:"
echo "# shasum -a 256 -c files.sha256" echo "# shasum -a 256 -c files.sha256"
echo "# The git tree hash above is the single value that covers all of it:" echo "# Hash checking alone ignores files this manifest does not list, and"
echo "# the Xcode project compiles any source file present in the tree. So"
echo "# also confirm nothing extra is present:"
echo "# git status --porcelain --ignored # git checkout: must print nothing"
echo "# or, for a tarball, diff this manifest's path list against find(1)."
echo "# Full instructions: docs/VERIFYING-A-BUILD.md"
echo "# The git tree hash above is the single value covering all tracked content:"
echo "# git rev-parse HEAD^{tree}" echo "# git rev-parse HEAD^{tree}"
echo "#" echo "#"
} > SOURCE-MANIFEST.txt } > SOURCE-MANIFEST.txt
+3
View File
@@ -848,6 +848,9 @@ final class NostrRelayManager: ObservableObject {
} }
} }
messageQueueLock.unlock() messageQueueLock.unlock()
// A relay queued while Tor bootstraps would otherwise reconnect when
// the queue drains, overriding the explicit removal.
pendingTorConnectionURLs.subtract(urls)
relays.removeAll { urls.contains($0.url) } relays.removeAll { urls.contains($0.url) }
updateConnectionStatus() updateConnectionStatus()
} }
@@ -64,6 +64,10 @@ extension ChatViewModel {
@objc func handleTorBootstrapDidStall() { @objc func handleTorBootstrapDidStall() {
Task { @MainActor in Task { @MainActor in
guard TorManager.shared.torEnforced else { return } guard TorManager.shared.torEnforced else { return }
// torEnforced is a compile-time constant in release builds; the
// runtime preference is what says whether anyone is waiting on
// Tor. Turning Tor off mid-bootstrap must not read as blocking.
guard NetworkActivationService.persistedTorPreference() else { return }
guard !self.torStallAnnounced else { return } guard !self.torStallAnnounced else { return }
self.torStallAnnounced = true self.torStallAnnounced = true
self.addGeohashOnlySystemMessage( self.addGeohashOnlySystemMessage(
@@ -1374,3 +1374,37 @@ private func makeImageData() throws -> Data {
return data return data
#endif #endif
} }
// MARK: - Tor Extension Tests
struct ChatViewModelTorExtensionTests {
/// Turning Tor off mid-bootstrap must not read as "the network is
/// blocking tor": `torEnforced` is a compile-time constant, so the stall
/// handler has to consult the runtime preference before announcing.
@Test @MainActor
func bootstrapStall_withTorPreferenceOff_announcesNothing() async {
let key = NetworkActivationService.torPreferenceKey
let previous = UserDefaults.standard.object(forKey: key)
defer {
if let previous {
UserDefaults.standard.set(previous, forKey: key)
} else {
UserDefaults.standard.removeObject(forKey: key)
}
}
let (viewModel, _) = makeTestableViewModel()
UserDefaults.standard.set(false, forKey: key)
viewModel.handleTorBootstrapDidStall()
try? await Task.sleep(nanoseconds: 50_000_000)
#expect(viewModel.torStallAnnounced == false)
// The same stall with the preference on (the persisted default) is
// exactly what must still be announced.
UserDefaults.standard.set(true, forKey: key)
viewModel.handleTorBootstrapDidStall()
try? await Task.sleep(nanoseconds: 50_000_000)
#expect(viewModel.torStallAnnounced == true)
}
}
@@ -44,6 +44,46 @@ final class NostrRelayManagerTests: XCTestCase {
XCTAssertTrue(context.sessionFactory.allConnections.allSatisfy { $0.cancelCallCount >= 1 }) XCTAssertTrue(context.sessionFactory.allConnections.allSatisfy { $0.cancelCallCount >= 1 })
} }
/// A relay removed while its connection is queued behind Tor bootstrap
/// must stay removed: draining the pending set used to resurrect it,
/// because `dropRelays` never touched `pendingTorConnectionURLs` and a
/// custom relay is in neither the default set nor the allow-list filter.
func test_relayRemovedWhileWaitingForTor_staysRemovedWhenTorBecomesReady() async {
let customURL = "wss://custom-removed.example"
let center = NotificationCenter()
let customRelays = MutableRelayList(urls: [customURL])
let context = makeContext(
permission: .authorized,
userTorEnabled: true,
torEnforced: true,
torIsReady: false,
notificationCenter: center,
customRelays: customRelays
)
// Defaults plus the custom relay all queue while Tor bootstraps.
context.manager.connect()
XCTAssertTrue(context.sessionFactory.requestedURLs.isEmpty)
XCTAssertEqual(context.torWaiter.awaitCallCount, 1)
// The relay is removed by hand before Tor is ready.
customRelays.urls = []
center.post(name: NostrRelaySettings.didChangeNotification, object: nil)
// The settings sink hops through the main queue; let it land.
try? await Task.sleep(nanoseconds: 20_000_000)
context.torWaiter.resolve(true)
let defaultsConnected = await waitUntil {
context.sessionFactory.requestedURLs.count == self.expectedDefaultRelayCount
}
XCTAssertTrue(defaultsConnected)
XCTAssertFalse(
context.sessionFactory.requestedURLs.contains(customURL),
"a relay removed while Tor was bootstrapping must not reconnect when the pending queue drains"
)
}
func test_connect_waitsForTorReadinessBeforeCreatingSessions() async { func test_connect_waitsForTorReadinessBeforeCreatingSessions() async {
let context = makeContext(permission: .authorized, userTorEnabled: true, torEnforced: true, torIsReady: false) let context = makeContext(permission: .authorized, userTorEnabled: true, torEnforced: true, torIsReady: false)
+21 -4
View File
@@ -18,26 +18,43 @@ In order of how much verification is possible:
Every tagged release has a `SOURCE-MANIFEST.txt` produced by `.github/workflows/source-manifest.yml`. It records the tag, the commit, the git tree hash, and a SHA-256 for every tracked file. Every tagged release has a `SOURCE-MANIFEST.txt` produced by `.github/workflows/source-manifest.yml`. It records the tag, the commit, the git tree hash, and a SHA-256 for every tracked file.
Check a copy of the source against it: Keep the downloaded manifest *outside* the source tree (say, `/tmp`) — a stray copy inside the checkout would itself trip the completeness checks below. Then check a copy of the source against it:
```sh ```sh
# From the root of the source you obtained # From the root of the source you obtained, with the manifest at /tmp
grep -v '^#' SOURCE-MANIFEST.txt > /tmp/files.sha256 grep -v '^#' /tmp/SOURCE-MANIFEST.txt > /tmp/files.sha256
shasum -a 256 -c /tmp/files.sha256 shasum -a 256 -c /tmp/files.sha256
``` ```
Any `FAILED` line means that file differs from the released source. Investigate before building. Any `FAILED` line means that file differs from the released source. Investigate before building.
That check alone is not enough. `shasum -c` verifies the files the manifest lists and says nothing about files it does not list — and the Xcode project compiles every source file present in the tree automatically, so a hostile mirror can pass the hash check by leaving every listed file intact and *adding* one. Confirm nothing extra is present:
```sh
# The manifest's path list must match the tree exactly — no missing files, no extras
grep -v '^#' /tmp/SOURCE-MANIFEST.txt | sed 's/^[0-9a-f]* //' | LC_ALL=C sort > /tmp/manifest-paths
find . -type f ! -path './.git/*' | sed 's|^\./||' | LC_ALL=C sort > /tmp/actual-paths
diff /tmp/manifest-paths /tmp/actual-paths # must print nothing
```
In a git checkout the same assurance is one command — it also catches extra files, because they show as untracked. `--ignored` matters: `.gitignore` covers paths like `build/`, plain `git status` would not report a planted file there, and Xcode compiles it all the same:
```sh
git status --porcelain --ignored # must print nothing before you build
```
The single value that covers the whole tree is the git tree hash in the manifest header: The single value that covers the whole tree is the git tree hash in the manifest header:
```sh ```sh
git rev-parse HEAD^{tree} # must equal the "tree:" line in the manifest git rev-parse HEAD^{tree} # must equal the "tree:" line in the manifest
``` ```
Note the tree hash covers tracked content only; it does not see untracked files sitting in the working directory, which is why the emptiness checks above come first.
The manifest itself carries a provenance attestation tying it to the workflow run that produced it, so a manifest handed to you along with a mirror is checkable too: The manifest itself carries a provenance attestation tying it to the workflow run that produced it, so a manifest handed to you along with a mirror is checkable too:
```sh ```sh
gh attestation verify SOURCE-MANIFEST.txt --repo permissionlesstech/bitchat gh attestation verify /tmp/SOURCE-MANIFEST.txt --repo permissionlesstech/bitchat
``` ```
That last step is what makes this resistant to a hostile mirror. Without it, whoever gives you the source can give you a matching manifest. That last step is what makes this resistant to a hostile mirror. Without it, whoever gives you the source can give you a matching manifest.
+15 -7
View File
@@ -84,6 +84,10 @@ public final class TorManager: ObservableObject {
private var shutdownsInFlight = 0 private var shutdownsInFlight = 0
private var startPendingAfterShutdown = false private var startPendingAfterShutdown = false
private var bootstrapMonitorStarted = false private var bootstrapMonitorStarted = false
// Fences the detached poll loop: shutdown, dormancy, and restart each bump
// this, so a loop from a previous attempt cannot run out its deadline and
// report a stall over state that a newer lifecycle event already owns.
private var bootstrapGeneration = 0
private var pathMonitor: NWPathMonitor? private var pathMonitor: NWPathMonitor?
private var isAppForeground: Bool = true private var isAppForeground: Bool = true
private var lastRestartAt: Date? = nil private var lastRestartAt: Date? = nil
@@ -268,24 +272,25 @@ public final class TorManager: ObservableObject {
private func startBootstrapMonitor() { private func startBootstrapMonitor() {
guard !bootstrapMonitorStarted else { return } guard !bootstrapMonitorStarted else { return }
bootstrapMonitorStarted = true bootstrapMonitorStarted = true
bootstrapGeneration += 1
let generation = bootstrapGeneration
Task.detached(priority: .utility) { [weak self] in Task.detached(priority: .utility) { [weak self] in
await self?.bootstrapPollLoop() await self?.bootstrapPollLoop(generation: generation)
} }
} }
private func bootstrapPollLoop() async { private func bootstrapPollLoop(generation: Int) async {
let deadline = Date().addingTimeInterval(75) let deadline = Date().addingTimeInterval(75)
var didComplete = false var didComplete = false
while Date() < deadline { while Date() < deadline {
guard generation == bootstrapGeneration else { return }
let progress = Int(arti_bootstrap_progress()) let progress = Int(arti_bootstrap_progress())
let summary = getBootstrapSummary() let summary = getBootstrapSummary()
await MainActor.run {
self.bootstrapProgress = progress self.bootstrapProgress = progress
self.bootstrapSummary = summary self.bootstrapSummary = summary
if progress >= 100 { self.isStarting = false } if progress >= 100 { self.isStarting = false }
self.recomputeReady() self.recomputeReady()
}
if progress >= 100 { if progress >= 100 {
didComplete = true didComplete = true
@@ -296,9 +301,10 @@ public final class TorManager: ObservableObject {
// Running out the deadline is a reportable outcome, not silence. The // Running out the deadline is a reportable outcome, not silence. The
// loop previously just ended, leaving `isStarting` true forever, so a // loop previously just ended, leaving `isStarting` true forever, so a
// blocked network was indistinguishable from a slow one. // blocked network was indistinguishable from a slow one. A deliberate
// shutdown mid-bootstrap is not a stall, hence the generation check.
if !didComplete { if !didComplete {
await MainActor.run { guard generation == bootstrapGeneration else { return }
self.isStarting = false self.isStarting = false
self.bootstrapDidStall = true self.bootstrapDidStall = true
SecureLogger.warning( SecureLogger.warning(
@@ -308,7 +314,6 @@ public final class TorManager: ObservableObject {
NotificationCenter.default.post(name: .TorBootstrapDidStall, object: nil) NotificationCenter.default.post(name: .TorBootstrapDidStall, object: nil)
} }
} }
}
private func getBootstrapSummary() -> String { private func getBootstrapSummary() -> String {
var buf = [CChar](repeating: 0, count: 256) var buf = [CChar](repeating: 0, count: 256)
@@ -352,6 +357,7 @@ public final class TorManager: ObservableObject {
// Clear isStarting so foreground recovery can proceed if bootstrap was interrupted. // Clear isStarting so foreground recovery can proceed if bootstrap was interrupted.
SecureLogger.debug("TorManager: goDormantOnBackground() called", category: .session) SecureLogger.debug("TorManager: goDormantOnBackground() called", category: .session)
Task { @MainActor in Task { @MainActor in
self.bootstrapGeneration += 1
self.isReady = false self.isReady = false
self.socksReady = false self.socksReady = false
self.isStarting = false self.isStarting = false
@@ -361,6 +367,7 @@ public final class TorManager: ObservableObject {
public func shutdownCompletely() { public func shutdownCompletely() {
SecureLogger.debug("TorManager: shutdownCompletely() called", category: .session) SecureLogger.debug("TorManager: shutdownCompletely() called", category: .session)
startPendingAfterShutdown = false startPendingAfterShutdown = false
bootstrapGeneration += 1
shutdownsInFlight += 1 shutdownsInFlight += 1
Task.detached { [weak self] in Task.detached { [weak self] in
guard let self = self else { return } guard let self = self else { return }
@@ -398,6 +405,7 @@ public final class TorManager: ObservableObject {
SecureLogger.debug("TorManager: restartArti() starting", category: .session) SecureLogger.debug("TorManager: restartArti() starting", category: .session)
await MainActor.run { await MainActor.run {
NotificationCenter.default.post(name: .TorWillRestart, object: nil) NotificationCenter.default.post(name: .TorWillRestart, object: nil)
self.bootstrapGeneration += 1
self.isReady = false self.isReady = false
self.socksReady = false self.socksReady = false
self.bootstrapProgress = 0 self.bootstrapProgress = 0