fix(app): a saved file is the own user's from the start, and replaces the one chosen whole

The reading of the full reading's fixes found that writePrivately made
the file, then set it to 0600: another local user could open it in that
window and keep reading what was written after; and that where the path
was a link or another user's file, chmod failed and the secret was
written into it anyway.

On Linux and macOS it now makes a new file beside the one chosen, 0600
as it is created (O_EXCL, O_NOFOLLOW), writes through the descriptor that
made it, and renames it over the chosen path, so the old file is never
written into; a symbolic link or another user's file is refused, with
nothing written (lstat through FFI, by each system's layout). Windows
writes in place, with its folder's rights.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Tizian 「ティツィアーン」 <raisondetredev@pm.me>

packages/halkyon_app/README.md

@@ -100,9 +100,12 @@ Written as the apps are: Dart 3.13's primary constructors, Material from
   compiler and make: sodium's build hook compiles libsodium.
 - **Files** (`FileSaver`, `PlatformFileSaver`): keeping a file the app made
   where the person chooses (a recovery phrase, recovery codes, an app's
-  exports): on the desktop the own user's alone (0600, `writePrivately`), on
-  iOS without the copy file_picker leaves in the temporary folder
-  (`forgetPickerCopy`).
+  exports): on Linux and macOS the own user's alone from the start
+  (`writePrivately`: a new file beside it, made 0600 with `O_EXCL`, written
+  through its descriptor, renamed over the one chosen, never written into;
+  a symbolic link or another user's file refused), on Windows written in
+  place with its folder's rights, on iOS without the copy file_picker leaves
+  in the temporary folder (`forgetPickerCopy`).
 - **Sync** (`lib/src/sync/`): `SyncService` (once at start, shortly after
   local changes, on resume and every few minutes, one sync at a time, counting
   the changes not on the server yet, `PendingChangesStore`), `AppCompatibility`

packages/halkyon_app/lib/src/files/file_saver.dart

@@ -1,6 +1,7 @@
 import 'dart:developer';
 import 'dart:ffi';
 import 'dart:io';
+import 'dart:math' show Random;
 import 'dart:typed_data';
 
 import 'package:ffi/ffi.dart';
@@ -58,17 +59,36 @@ class PlatformFileSaver() implements FileSaver {
   }
 }
 
-/// Writes [bytes] to [path], where the platform has modes (Linux, macOS)
-/// readable and writable by the own user alone (0600), set on the emptied
-/// file before anything is written, so no other user of the machine ever
-/// reads it. Windows' files keep its folders' rights.
+/// Writes [bytes] to [path] so that nobody but the own user ever reads them
+/// (the reading of the fixes), where the platform has modes (Linux, macOS):
+/// into a new file beside it, made 0600 as it is created (`O_EXCL`, so
+/// never one another user made first) and written through the descriptor
+/// that made it, then renamed over [path]. So the file the person chose is
+/// replaced whole, never written into: whoever held the old one open, or a
+/// link to it, reads only what it held. Refused ([FileSystemException]),
+/// with nothing written, where [path] is a symbolic link (never followed)
+/// or a file another user owns. Windows' files keep their folder's rights,
+/// a user's own folders being theirs alone, and are written in place.
 Future<void> writePrivately(String path, Uint8List bytes) async {
-  final file = File(path);
-  await file.writeAsBytes(const [], flush: true);
-  if (Platform.isLinux || Platform.isMacOS) {
-    _ownUserOnly(path);
+  if (!Platform.isLinux && !Platform.isMacOS) {
+    await File(path).writeAsBytes(bytes, flush: true);
+    return;
+  }
+  _refuseLinkOrOthers(path);
+  final temporary = p.join(
+    p.dirname(path),
+    '.${p.basename(path)}.${_randomHex()}.tmp',
+  );
+  try {
+    _writeNew(temporary, bytes);
+    // Checked once more: a link put there meanwhile is replaced by the
+    // rename, never followed, but the person is told.
+    _refuseLinkOrOthers(path);
+    _posix.rename(temporary, path);
+  } on Object {
+    _posix.unlinkQuietly(temporary);
+    rethrow;
   }
-  await file.writeAsBytes(bytes, flush: true);
 }
 
 /// Removes the copy of the file named [name] that file_picker (checked with
@@ -90,22 +110,196 @@ Future<void> forgetPickerCopy(String name, {Directory? temporary}) async {
   }
 }
 
-typedef _ChmodNative = Int32 Function(Pointer<Utf8> path, Uint32 mode);
-typedef _Chmod = int Function(Pointer<Utf8> path, int mode);
+String _randomHex() {
+  final random = Random.secure();
+  return [
+    for (var i = 0; i < 8; i++)
+      random.nextInt(256).toRadixString(16).padLeft(2, '0'),
+  ].join();
+}
 
-/// chmod(2) of [path] to 0600, through the C library already in the
-/// process: dart:io sets no modes, and running chmod(1) would leave a
-/// sandboxed macOS app without the file's access.
-void _ownUserOnly(String path) {
-  final chmod = DynamicLibrary.process().lookupFunction<_ChmodNative, _Chmod>(
-    'chmod',
+/// Refuses [path] when it is a symbolic link or a file another user owns;
+/// nothing there passes.
+void _refuseLinkOrOthers(String path) {
+  final found = _posix.lstat(path);
+  if (found == null) {
+    return;
+  }
+  if (found.isLink) {
+    throw FileSystemException('A symbolic link: never followed', path);
+  }
+  if (found.owner != _posix.geteuid()) {
+    throw FileSystemException("Another user's file", path);
+  }
+}
+
+/// Makes [path], which mustn't exist, 0600 as it is created, and writes
+/// [bytes] through the descriptor that made it.
+void _writeNew(String path, Uint8List bytes) {
+  final fd = _posix.openNew(path);
+  try {
+    final buffer = malloc<Uint8>(bytes.isEmpty ? 1 : bytes.length);
+    try {
+      buffer.asTypedList(bytes.length).setAll(0, bytes);
+      var written = 0;
+      while (written < bytes.length) {
+        final wrote = _posix.write(
+          fd,
+          buffer + written,
+          bytes.length - written,
         );
+        if (wrote <= 0) {
+          throw FileSystemException('Writing failed', path);
+        }
+        written += wrote;
+      }
+      // Wiped, as it may hold a secret.
+      buffer.asTypedList(bytes.length).fillRange(0, bytes.length, 0);
+    } finally {
+      malloc.free(buffer);
+    }
+    if (_posix.fsync(fd) != 0) {
+      throw FileSystemException('Flushing failed', path);
+    }
+  } finally {
+    _posix.close(fd);
+  }
+}
+
+final _posix = _Posix();
+
+/// The few calls of the C library [writePrivately] needs, through the
+/// library already in the process: dart:io sets no modes, creates nothing
+/// exclusively, and running chmod(1) would leave a sandboxed macOS app
+/// without the file's access.
+final class _Posix() {
+  final DynamicLibrary _library = DynamicLibrary.process();
+
+  late final _open = _library
+      .lookupFunction<
+        Int32 Function(Pointer<Utf8>, Int32, Uint32),
+        int Function(Pointer<Utf8>, int, int)
+      >('open');
+  late final write = _library
+      .lookupFunction<
+        IntPtr Function(Int32, Pointer<Uint8>, IntPtr),
+        int Function(int, Pointer<Uint8>, int)
+      >('write');
+  late final fsync = _library
+      .lookupFunction<Int32 Function(Int32), int Function(int)>('fsync');
+  late final close = _library
+      .lookupFunction<Int32 Function(Int32), int Function(int)>('close');
+  late final _rename = _library
+      .lookupFunction<
+        Int32 Function(Pointer<Utf8>, Pointer<Utf8>),
+        int Function(Pointer<Utf8>, Pointer<Utf8>)
+      >('rename');
+  late final _unlink = _library
+      .lookupFunction<
+        Int32 Function(Pointer<Utf8>),
+        int Function(Pointer<Utf8>)
+      >('unlink');
+  late final geteuid = _library
+      .lookupFunction<Uint32 Function(), int Function()>('geteuid');
+
+  /// `O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC`, as each
+  /// system numbers them.
+  int get _newFileFlags => Platform.isMacOS
+      ? 0x1 | 0x200 | 0x800 | 0x100 | 0x1000000
+      : 0x1 | 0x40 | 0x80 | _linuxNoFollow | 0x80000;
+
+  /// `O_NOFOLLOW`: Arm's Linux numbers it otherwise than the generic one.
+  int get _linuxNoFollow => Abi.current() == Abi.linuxArm64 ? 0x8000 : 0x20000;
+
+  int openNew(String path) {
     final native = path.toNativeUtf8();
     try {
-    if (chmod(native, 0x180) != 0) {
-      log('The saved file keeps its mode', name: 'Files');
+      final fd = _open(native, _newFileFlags, 0x180);
+      if (fd < 0) {
+        throw FileSystemException('Creating it failed', path);
       }
+      return fd;
     } finally {
       malloc.free(native);
     }
+  }
+
+  void rename(String from, String to) {
+    final nativeFrom = from.toNativeUtf8();
+    final nativeTo = to.toNativeUtf8();
+    try {
+      if (_rename(nativeFrom, nativeTo) != 0) {
+        throw FileSystemException('Renaming failed', to);
+      }
+    } finally {
+      malloc
+        ..free(nativeFrom)
+        ..free(nativeTo);
+    }
+  }
+
+  void unlinkQuietly(String path) {
+    final native = path.toNativeUtf8();
+    try {
+      _unlink(native);
+    } finally {
+      malloc.free(native);
+    }
+  }
+
+  /// [path]'s type and owner, not following a link; null where nothing is.
+  ({bool isLink, int owner})? lstat(String path) {
+    // Where each system's `struct stat` keeps the mode and the owner.
+    final (:mode, :modeBytes, :owner) = switch (Abi.current()) {
+      Abi.macosArm64 || Abi.macosX64 => (mode: 4, modeBytes: 2, owner: 16),
+      Abi.linuxX64 => (mode: 24, modeBytes: 4, owner: 28),
+      // The kernel's generic layout.
+      Abi.linuxArm64 || Abi.linuxRiscv64 => (mode: 16, modeBytes: 4, owner: 24),
+      _ => throw FileSystemException('Its owner is unknown here', path),
+    };
+    final native = path.toNativeUtf8();
+    final buffer = calloc<Uint8>(256);
+    try {
+      if (_lstat(native, buffer) != 0) {
+        // Anything but "there is none" refuses.
+        if (FileSystemEntity.typeSync(path, followLinks: false) ==
+            FileSystemEntityType.notFound) {
+          return null;
+        }
+        throw FileSystemException('Its owner is unknown', path);
+      }
+      final view = ByteData.sublistView(buffer.asTypedList(256));
+      final fileMode = modeBytes == 2
+          ? view.getUint16(mode, Endian.host)
+          : view.getUint32(mode, Endian.host);
+      return (
+        isLink: fileMode & 0xf000 == 0xa000,
+        owner: view.getUint32(owner, Endian.host),
+      );
+    } finally {
+      calloc.free(buffer);
+      malloc.free(native);
+    }
+  }
+
+  /// lstat(2): on Intel Macs its 64-bit inode variant, whose layout Arm's
+  /// has; on a glibc older than 2.33, `__lxstat`.
+  late final int Function(Pointer<Utf8>, Pointer<Uint8>) _lstat = () {
+    for (final name in [if (Platform.isMacOS) r'lstat$INODE64', 'lstat']) {
+      if (_library.providesSymbol(name)) {
+        return _library.lookupFunction<
+          Int32 Function(Pointer<Utf8>, Pointer<Uint8>),
+          int Function(Pointer<Utf8>, Pointer<Uint8>)
+        >(name);
+      }
+    }
+    final lxstat = _library
+        .lookupFunction<
+          Int32 Function(Int32, Pointer<Utf8>, Pointer<Uint8>),
+          int Function(int, Pointer<Utf8>, Pointer<Uint8>)
+        >('__lxstat');
+    final version = Abi.current() == Abi.linuxArm64 ? 0 : 1;
+    return (Pointer<Utf8> path, Pointer<Uint8> buffer) =>
+        lxstat(version, path, buffer);
+  }();
 }

packages/halkyon_app/test/files/file_saver_test.dart

@@ -1,3 +1,4 @@
+import 'dart:convert';
 import 'dart:io';
 import 'dart:typed_data';
 
@@ -30,6 +31,85 @@ void main() {
     }
   }, skip: Platform.isWindows ? 'no modes' : false);
 
+  final hasModes = Platform.isLinux || Platform.isMacOS;
+
+  test('a saved file replaces the one there without writing into it: '
+      'whoever held the old file open, or a link to it, never reads what is '
+      'saved (the reading of the fixes)', () async {
+    final path = p.join(folder.path, 'halkyon-recovery-codes.txt');
+    await File(path).writeAsString('old');
+    await Process.run('chmod', ['644', path]);
+    // Another name for the same file, as a descriptor opened earlier is.
+    final elsewhere = p.join(folder.path, 'held-open');
+    await Process.run('ln', [path, elsewhere]);
+
+    await writePrivately(path, Uint8List.fromList(utf8.encode('secret')));
+
+    expect(await File(path).readAsString(), 'secret');
+    expect(File(path).statSync().mode & 0x1ff, 0x180, reason: '0600');
+    expect(await File(elsewhere).readAsString(), 'old');
+    // Nothing left beside it.
+    expect(
+      folder.listSync().map((entry) => p.basename(entry.path)).toSet(),
+      {'halkyon-recovery-codes.txt', 'held-open'},
+    );
+  }, skip: hasModes ? false : 'no modes');
+
+  test('never follows a symbolic link where the file is to be saved: '
+      'refused, the file it points to untouched', () async {
+    final target = p.join(folder.path, 'someone-elses.txt');
+    await File(target).writeAsString('theirs');
+    final path = p.join(folder.path, 'halkyon-recovery-phrase.txt');
+    await Link(path).create(target);
+
+    await expectLater(
+      writePrivately(path, Uint8List.fromList(utf8.encode('secret'))),
+      throwsA(isA<FileSystemException>()),
+    );
+
+    expect(await File(target).readAsString(), 'theirs');
+    expect(FileSystemEntity.isLinkSync(path), isTrue);
+    expect(
+      folder.listSync().map((entry) => p.basename(entry.path)).toSet(),
+      {'someone-elses.txt', 'halkyon-recovery-phrase.txt'},
+    );
+  }, skip: hasModes ? false : 'no modes');
+
+  test(
+    "refuses a file another user owns, writing nothing: a root's file "
+    'stands in for one in a shared folder',
+    () async {
+      const rootsFile = '/etc/hostname';
+      final before = File(rootsFile).readAsStringSync();
+
+      await expectLater(
+        writePrivately(rootsFile, Uint8List.fromList([1])),
+        throwsA(
+          isA<FileSystemException>().having(
+            (error) => error.message,
+            'message',
+            "Another user's file",
+          ),
+        ),
+      );
+      expect(File(rootsFile).readAsStringSync(), before);
+    },
+    skip: !hasModes || !File('/etc/hostname').existsSync()
+        ? 'no such file'
+        : false,
+  );
+
+  test('a new file is made 0600 from the start, with nothing left beside '
+      'it', () async {
+    final path = p.join(folder.path, 'new.txt');
+
+    await writePrivately(path, Uint8List.fromList([4, 5, 6]));
+
+    expect(await File(path).readAsBytes(), [4, 5, 6]);
+    expect(File(path).statSync().mode & 0x1ff, 0x180, reason: '0600');
+    expect(folder.listSync(), hasLength(1));
+  }, skip: hasModes ? false : 'no modes');
+
   test("the system picker's copy in the temporary folder is removed, "
       'others left', () async {
     final copy = File(p.join(folder.path, 'halkyon-recovery-phrase.txt'));