From 86e96e5ca6006fe6a057a4bb833498c9d23c3b5b Mon Sep 17 00:00:00 2001 From: Joachim Rosskopf Date: Fri, 22 May 2026 12:22:13 +0200 Subject: [PATCH 1/2] feat: macOS notarised self-packaging via reserved Mach-O segment Part of #40. Closes #48. macOS-notarised distribution requires a stable binary signature. The Linux/Windows "append a ZIP after EOF" trick invalidates the signature because trailing bytes after __LINKEDIT aren't covered. This PR introduces the reserved-segment approach used by AppImage, PyInstaller, and friends: 1. The macOS build allocates a placeholder __FLAPI/__bundle Mach-O section at link time (default 16 MiB, knob FLAPI_RESERVED_BUNDLE_MIB). 2. `flapi pack` overwrites the segment in place rather than appending after EOF. 3. `pack` re-invokes `codesign` after writing, so the freshly bundled binary has a fresh, valid signature. 4. The runtime locator looks for the reserved segment first; Linux/Windows binaries don't have one, so behaviour there is unchanged. Implementation: - src/include/macho_bundle.hpp + src/macho_bundle.cpp -- 64-bit Mach-O parser (header + LC_SEGMENT_64 + section_64), no external deps, compiles on all platforms. CodesignBinary() wraps popen for the macOS path and is a benign no-op elsewhere. - CMakeLists.txt (APPLE branch only) -- adds the placeholder file via add_custom_command + dd, and links `flapi` with -Wl,-sectcreate,__FLAPI,__bundle,. Linux/Windows builds skip this entirely. - src/bundle_locator.{hpp,cpp} -- refactored. Extracted the EOCD reverse-scan into `ScanBufferForEocd`, shared by: * `LocateBundle(path)` -- existing EOF-tail scan * `LocateBundleInRange(path, off, size)` -- NEW. Scans a sub-range of the file. Used by section-mode lookup. * `LocateBundleInSelf()` -- NEW behaviour: try the macOS section first, fall back to EOF tail. - src/include/pack.hpp -- new enum `MacOSPackMode` + PackOptions fields `macos_mode` (defaults to kReservedSegment) and `codesign` (defaults to true; tests turn it off to skip the codesign call). - src/pack.cpp -- Pack() probes for the Mach-O section first; if present, copies the host whole and `OverwriteFlapiSection` writes the archive in place. If the section is absent (Linux/Windows) OR `--macos-append` was passed, falls through to the existing append-after-EOF code path. Re-signs on Darwin afterwards. - src/main.cpp -- new `--macos-append` flag on the `pack` subcommand. Threads through to PackOptions. Tests: - test/cpp/macho_bundle_test.cpp -- 7 cases / 13 assertions. Builds synthetic Mach-O fixtures byte-by-byte and feeds them to `LocateFlapiSectionInBuffer`. Covers: magic recognition, present section, absent section, wrong-segment name match attempt, short buffer, non-Mach-O input, CodesignBinary no-op on non-Darwin. - test/integration/test_self_packaging_macos.py -- 4 cases, all marked `pytest.mark.skipif(platform.system() != "Darwin")`: 1. unbundled flapi has the reserved __FLAPI/__bundle segment (otool -l check) 2. default pack passes `codesign --verify --strict` 3. `--macos-append` produces a runnable artifact with a discoverable bundle 4. oversized payload (32 MiB into a 16 MiB segment) is rejected with an error mentioning FLAPI_RESERVED_BUNDLE_MIB - All existing 14 Linux integration tests still pass; the new code is fully backward-compatible (section probe returns nullopt on Linux, code falls through to the existing append path). - 23/23 C++ unit tests in the [pack],[macho_bundle],[bundle_locator] tag set pass (62 assertions). Out of scope (deferred to follow-ups): - Fat (universal) binary support -- the parser handles thin 64-bit Mach-O only. macOS releases produced by this repo are per-architecture thin so the gap is acceptable for now. - 32-bit Mach-O. Same reasoning. Closes #48. Part of #40. --- CMakeLists.txt | 23 ++ src/bundle_locator.cpp | 175 +++++---- src/include/bundle_locator.hpp | 16 +- src/include/macho_bundle.hpp | 72 ++++ src/include/pack.hpp | 20 + src/macho_bundle.cpp | 341 ++++++++++++++++++ src/main.cpp | 9 + src/pack.cpp | 58 ++- test/cpp/CMakeLists.txt | 1 + test/cpp/macho_bundle_test.cpp | 180 +++++++++ test/integration/test_self_packaging_macos.py | 143 ++++++++ 11 files changed, 968 insertions(+), 70 deletions(-) create mode 100644 src/include/macho_bundle.hpp create mode 100644 src/macho_bundle.cpp create mode 100644 test/cpp/macho_bundle_test.cpp create mode 100644 test/integration/test_self_packaging_macos.py diff --git a/CMakeLists.txt b/CMakeLists.txt index 416f444..f975a39 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -250,6 +250,7 @@ add_library(flapi-lib STATIC src/endpoint_config_parser.cpp src/extended_yaml_parser.cpp src/heartbeat_worker.cpp + src/macho_bundle.cpp src/open_api_doc_generator.cpp src/pack.cpp src/password_hasher.cpp @@ -344,6 +345,28 @@ target_link_libraries(flapi PRIVATE flapi-lib) target_compile_definitions(flapi PRIVATE FLAPI_VERSION="${CMAKE_PROJECT_VERSION}") set_target_properties(flapi PROPERTIES ENABLE_EXPORTS TRUE) +# macOS reserved-segment for self-packaging (#48). Allocates a +# placeholder __FLAPI/__bundle section at link time that `flapi pack` +# overwrites with the user's config tree, then re-signs. Required for +# notarised distribution -- appending after __LINKEDIT invalidates +# the signature. +if(APPLE) + set(FLAPI_RESERVED_BUNDLE_MIB "16" CACHE STRING + "Size in MiB of the reserved __FLAPI/__bundle Mach-O segment.") + set(FLAPI_BUNDLE_PLACEHOLDER + "${CMAKE_BINARY_DIR}/_flapi_bundle_placeholder.bin") + add_custom_command( + OUTPUT "${FLAPI_BUNDLE_PLACEHOLDER}" + COMMAND ${CMAKE_COMMAND} -E env + bash -c "dd if=/dev/zero of='${FLAPI_BUNDLE_PLACEHOLDER}' bs=1m count=${FLAPI_RESERVED_BUNDLE_MIB} >/dev/null 2>&1" + COMMENT "flapi: generating ${FLAPI_RESERVED_BUNDLE_MIB}-MiB __FLAPI/__bundle placeholder") + add_custom_target(flapi_bundle_placeholder + DEPENDS "${FLAPI_BUNDLE_PLACEHOLDER}") + target_link_options(flapi PRIVATE + "-Wl,-sectcreate,__FLAPI,__bundle,${FLAPI_BUNDLE_PLACEHOLDER}") + add_dependencies(flapi flapi_bundle_placeholder) +endif() + # Add Windows-specific libraries if(WIN32) target_link_libraries(flapi PRIVATE Dbghelp.lib) diff --git a/src/bundle_locator.cpp b/src/bundle_locator.cpp index 9f404c6..30d0b7d 100644 --- a/src/bundle_locator.cpp +++ b/src/bundle_locator.cpp @@ -1,4 +1,5 @@ #include "bundle_locator.hpp" +#include "macho_bundle.hpp" #include "selfpath.hpp" #include @@ -14,8 +15,8 @@ namespace { constexpr std::size_t kEocdRecordSize = 22; constexpr std::size_t kMaxCommentLen = 0xffffu; -// We accept padding well in excess of the 10 KiB spike default; the -// total tail buffer is EOCD + max comment + 64 KiB pad budget. +// Total tail buffer = EOCD + max comment + 64 KiB pad budget. Generous +// vs. the spike's 10 KiB libarchive tar-block default; cheap to read. constexpr std::size_t kPaddingBudget = 65536; constexpr std::size_t kScanBudget = kEocdRecordSize + kMaxCommentLen + kPaddingBudget; @@ -32,57 +33,34 @@ std::uint32_t ReadU32(const std::uint8_t* p) { | (static_cast(p[3]) << 24); } -} // namespace - -std::optional LocateBundle(const std::filesystem::path& path) { - std::error_code ec; - const auto file_size = std::filesystem::file_size(path, ec); - if (ec || file_size < kEocdRecordSize) { - return std::nullopt; - } - - std::ifstream in(path, std::ios::binary); - if (!in.is_open()) { - return std::nullopt; - } - - const std::size_t tail_bytes = - static_cast(std::min(file_size, kScanBudget)); - const std::uint64_t tail_start = file_size - tail_bytes; - - std::vector tail(tail_bytes); - in.seekg(static_cast(tail_start), std::ios::beg); - in.read(reinterpret_cast(tail.data()), - static_cast(tail_bytes)); - if (!in) { - return std::nullopt; - } - if (tail.size() < kEocdRecordSize) { +// Scan a buffer for the most-recent valid EOCD record. `buf_start_in_file` +// is the absolute file offset of buf[0]; used to translate the result. +// `logical_eof_in_buf` is the buf-relative position to treat as EOF for +// padding-tolerance purposes (== buf.size() in the common case). +std::optional ScanBufferForEocd( + const std::vector& buf, + std::uint64_t buf_start_in_file, + std::size_t logical_eof_in_buf) { + if (logical_eof_in_buf > buf.size() || logical_eof_in_buf < kEocdRecordSize) { return std::nullopt; } - - // Reverse-scan from the latest valid signature position. The latest - // (largest-offset) EOCD wins, since any earlier signature byte - // sequence in random leading data is a false positive. - const std::size_t max_start = tail.size() - kEocdRecordSize; + const std::size_t max_start = logical_eof_in_buf - kEocdRecordSize; for (std::size_t i = max_start + 1; i-- > 0; ) { - if (tail[i] != 0x50 || - tail[i + 1] != 0x4b || - tail[i + 2] != 0x05 || - tail[i + 3] != 0x06) { + if (buf[i] != 0x50 || + buf[i + 1] != 0x4b || + buf[i + 2] != 0x05 || + buf[i + 3] != 0x06) { continue; } + const std::uint8_t* p = buf.data() + i; + const std::uint16_t this_disk = ReadU16(p + 4); + const std::uint16_t cd_start_disk = ReadU16(p + 6); + const std::uint16_t entries_this = ReadU16(p + 8); + const std::uint16_t entries_total = ReadU16(p + 10); + const std::uint32_t cd_size = ReadU32(p + 12); + const std::uint32_t cd_offset_arch = ReadU32(p + 16); + const std::uint16_t comment_len = ReadU16(p + 20); - const std::uint8_t* p = tail.data() + i; - const std::uint16_t this_disk = ReadU16(p + 4); - const std::uint16_t cd_start_disk = ReadU16(p + 6); - const std::uint16_t entries_this = ReadU16(p + 8); - const std::uint16_t entries_total = ReadU16(p + 10); - const std::uint32_t cd_size = ReadU32(p + 12); - const std::uint32_t cd_offset_arch = ReadU32(p + 16); - const std::uint16_t comment_len = ReadU16(p + 20); - - // Multi-disk archives are not supported. if (this_disk != 0 || cd_start_disk != 0) { continue; } @@ -90,17 +68,15 @@ std::optional LocateBundle(const std::filesystem::path& path) { continue; } - // The comment must fit in the tail. const std::size_t comment_end = i + kEocdRecordSize + comment_len; - if (comment_end > tail.size()) { + if (comment_end > logical_eof_in_buf) { continue; } - // Anything after the comment up to file-EOF must be zero - // padding -- the libarchive tar-block rounding tolerance. + // Anything after the comment up to logical EOF must be zero. bool padding_ok = true; - for (std::size_t j = comment_end; j < tail.size(); ++j) { - if (tail[j] != 0) { + for (std::size_t j = comment_end; j < logical_eof_in_buf; ++j) { + if (buf[j] != 0) { padding_ok = false; break; } @@ -109,22 +85,19 @@ std::optional LocateBundle(const std::filesystem::path& path) { continue; } - const std::uint64_t eocd_file_offset = tail_start + i; - - // The central directory sits immediately before the EOCD. + const std::uint64_t eocd_file_offset = buf_start_in_file + i; if (cd_size > eocd_file_offset) { continue; } const std::uint64_t cd_file_offset = eocd_file_offset - cd_size; - if (cd_offset_arch > cd_file_offset) { continue; } const std::uint64_t bundle_start = cd_file_offset - cd_offset_arch; - - const std::uint64_t bundle_end = eocd_file_offset + kEocdRecordSize + comment_len; + const std::uint64_t bundle_end = + eocd_file_offset + kEocdRecordSize + comment_len; if (bundle_end < bundle_start) { - continue; // overflow paranoia + continue; } BundleLocation loc; @@ -132,13 +105,91 @@ std::optional LocateBundle(const std::filesystem::path& path) { loc.size = bundle_end - bundle_start; return loc; } - return std::nullopt; } +} // namespace + +std::optional LocateBundle(const std::filesystem::path& path) { + std::error_code ec; + const auto file_size = std::filesystem::file_size(path, ec); + if (ec || file_size < kEocdRecordSize) { + return std::nullopt; + } + + std::ifstream in(path, std::ios::binary); + if (!in.is_open()) { + return std::nullopt; + } + + const std::size_t tail_bytes = + static_cast(std::min(file_size, kScanBudget)); + const std::uint64_t tail_start = file_size - tail_bytes; + + std::vector tail(tail_bytes); + in.seekg(static_cast(tail_start), std::ios::beg); + in.read(reinterpret_cast(tail.data()), + static_cast(tail_bytes)); + if (!in) { + return std::nullopt; + } + return ScanBufferForEocd(tail, tail_start, tail.size()); +} + +std::optional LocateBundleInRange( + const std::filesystem::path& path, + std::uint64_t range_offset, + std::uint64_t range_size) { + if (range_size < kEocdRecordSize) { + return std::nullopt; + } + std::error_code ec; + const auto file_size = std::filesystem::file_size(path, ec); + if (ec || range_offset + range_size > file_size) { + return std::nullopt; + } + // Cap range_size at something sensible -- a runaway section_size + // value from a malformed Mach-O could otherwise allocate gigabytes. + constexpr std::uint64_t kMaxRangeBytes = 64ull * 1024ull * 1024ull; + const std::size_t to_read = + static_cast(std::min(range_size, kMaxRangeBytes)); + + std::ifstream in(path, std::ios::binary); + if (!in.is_open()) { + return std::nullopt; + } + std::vector buf(to_read); + in.seekg(static_cast(range_offset), std::ios::beg); + in.read(reinterpret_cast(buf.data()), + static_cast(to_read)); + if (!in) { + return std::nullopt; + } + return ScanBufferForEocd(buf, range_offset, buf.size()); +} + std::optional LocateBundleInSelf() { + std::filesystem::path self_path; + try { + self_path = GetSelfPath(); + } catch (...) { + return std::nullopt; + } + + // Prefer the reserved Mach-O section if present (#48). Linux/Windows + // binaries lack the section, so this returns nullopt and we fall + // through to the EOF-tail scan unchanged. + if (auto sect = LocateFlapiSection(self_path); sect.has_value()) { + if (auto loc = LocateBundleInRange(self_path, sect->file_offset, sect->size); + loc.has_value()) { + return loc; + } + // Section is present but empty / unpopulated (e.g., un-packed + // build): fall through to the EOF-tail scan. + } + try { - return LocateBundle(GetSelfPath()); + return LocateBundle(self_path); } catch (...) { return std::nullopt; } diff --git a/src/include/bundle_locator.hpp b/src/include/bundle_locator.hpp index 4901c6b..78086b5 100644 --- a/src/include/bundle_locator.hpp +++ b/src/include/bundle_locator.hpp @@ -27,8 +27,20 @@ struct BundleLocation { // rounding pushing the EOCD off file-EOF. std::optional LocateBundle(const std::filesystem::path& path); -// Convenience: scan the currently running executable. Returns nullopt -// if either the self-path lookup or the EOCD scan fails. +// Scans a specific byte range of `path` for a ZIP EOCD. Used by the +// macOS section-mode locator (#48) where the bundle lives inside a +// reserved Mach-O segment, not at file EOF. The returned +// BundleLocation.offset is the absolute file offset; padding-tolerance +// runs against `range_size` (treat range end as logical EOF). +std::optional LocateBundleInRange( + const std::filesystem::path& path, + std::uint64_t range_offset, + std::uint64_t range_size); + +// Convenience: scan the currently running executable. On macOS, first +// looks at the reserved __FLAPI/__bundle Mach-O section; if absent or +// unpopulated, falls back to a reverse-EOF scan. Returns nullopt if +// either fails. std::optional LocateBundleInSelf(); } // namespace flapi diff --git a/src/include/macho_bundle.hpp b/src/include/macho_bundle.hpp new file mode 100644 index 0000000..5467df1 --- /dev/null +++ b/src/include/macho_bundle.hpp @@ -0,0 +1,72 @@ +#pragma once + +#include +#include +#include +#include +#include + +namespace flapi { + +// The reserved Mach-O segment+section we use to host the bundled +// ZIP on macOS. Allocated at link time with a placeholder of +// FLAPI_RESERVED_BUNDLE_MIB MiB and overwritten by `flapi pack` +// at packaging time so the binary signature stays valid after a +// re-`codesign`. +constexpr const char* kFlapiSegName = "__FLAPI"; +constexpr const char* kFlapiSectName = "__bundle"; + +struct MachOSection { + // File offset of the section's first byte, suitable for seek(). + std::uint64_t file_offset = 0; + // Allocated section size in bytes (== reserved capacity). + std::uint64_t size = 0; +}; + +// Returns true if the bytes at `magic_bytes` (read from the head of +// a file) are a Mach-O magic value we recognise. Cheap pre-check. +bool IsMachOMagic(const std::uint8_t magic_bytes[4]); + +// Locate the __FLAPI/__bundle section in a Mach-O file on disk. +// Returns nullopt if: +// - the file isn't a thin (non-fat) Mach-O, +// - the file is malformed, +// - the section doesn't exist (e.g., on a non-macOS build). +// +// Fat / universal binaries are currently not supported -- a follow-up +// can iterate slices. macOS releases produced by this repo are thin +// per-architecture, so the gap is acceptable for now. +std::optional LocateFlapiSection(const std::filesystem::path& path); + +// Overload that scans a buffer instead of opening a file. Used by +// unit tests against synthetic Mach-O fixtures. +std::optional LocateFlapiSectionInBuffer( + const std::vector& buffer); + +// Overwrite the reserved section at `binary` with `payload`. The +// trailing capacity beyond payload.size() is zero-padded so that the +// EOCD of the embedded ZIP still reverse-scans cleanly from segment +// EOF. Throws ArchiveIOError when the payload is bigger than the +// reserved capacity (see PackOptions::host_binary_override for the +// override knob). +// +// Caller is responsible for re-signing the binary afterwards +// (CodesignBinary), since the signature covers the section bytes. +void OverwriteFlapiSection(const std::filesystem::path& binary, + const MachOSection& section, + const std::vector& payload); + +// Result of a codesign attempt. +struct CodesignResult { + int exit_code = 0; + std::string identity; // "-" for ad-hoc; otherwise the CODESIGN_IDENTITY value + std::string stderr_tail; // last few KB of codesign stderr, for diagnostics +}; + +// Invoke `codesign --force --sign ` where identity +// is taken from the CODESIGN_IDENTITY env var, defaulting to "-" +// (ad-hoc). On non-Darwin builds this is a no-op that returns +// exit_code = 0 -- the caller doesn't need a platform guard. +CodesignResult CodesignBinary(const std::filesystem::path& binary); + +} // namespace flapi diff --git a/src/include/pack.hpp b/src/include/pack.hpp index 67a2418..d3dc3b4 100644 --- a/src/include/pack.hpp +++ b/src/include/pack.hpp @@ -16,6 +16,16 @@ class PackError : public std::runtime_error { explicit PackError(const std::string& m) : std::runtime_error(m) {} }; +enum class MacOSPackMode { + // Default on macOS: overwrite the reserved __FLAPI/__bundle Mach-O + // section in place and re-`codesign`. Produces notarisable output. + kReservedSegment, + // Legacy: append the archive after __LINKEDIT (the + // Linux/Windows path). Result is *not* notarisable -- ad-hoc + // signing only. Kept for compatibility / debugging. + kAppend, +}; + struct PackOptions { // Bundle files even when their relative path matches the default // secret-exclude list. Intended for tests; production users must @@ -33,6 +43,16 @@ struct PackOptions { // binary -- avoiding the cost of copying the multi-GB // sanitiser-instrumented test binary on every test case. std::optional host_binary_override; + + // macOS-only: which packaging mode to use. Defaults to + // kReservedSegment (notarisable). On Linux/Windows builds this + // field is ignored. + MacOSPackMode macos_mode = MacOSPackMode::kReservedSegment; + + // Re-sign the output via `codesign` after writing. Only honoured + // on Darwin (silent no-op elsewhere). Defaults to true; tests set + // this to false to skip the codesign call. + bool codesign = true; }; struct PackResult { diff --git a/src/macho_bundle.cpp b/src/macho_bundle.cpp new file mode 100644 index 0000000..c8af39b --- /dev/null +++ b/src/macho_bundle.cpp @@ -0,0 +1,341 @@ +#include "macho_bundle.hpp" + +#include "archive_io.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include + +#ifndef _WIN32 + #include + #include +#endif + +namespace flapi { + +namespace { + +// Magic constants from . Duplicated here so the +// parser is portable to Linux/Windows builds (the integration tests +// for Mach-O parsing on those platforms run against synthetic +// fixtures). +constexpr std::uint32_t kMachOMagic32 = 0xFEEDFACEu; +constexpr std::uint32_t kMachOCigam32 = 0xCEFAEDFEu; // byte-swapped +constexpr std::uint32_t kMachOMagic64 = 0xFEEDFACFu; +constexpr std::uint32_t kMachOCigam64 = 0xCFFAEDFEu; +constexpr std::uint32_t kFatMagic = 0xCAFEBABEu; +constexpr std::uint32_t kFatCigam = 0xBEBAFECAu; + +constexpr std::uint32_t kLcSegment = 0x01; +constexpr std::uint32_t kLcSegment64 = 0x19; + +std::uint32_t ReadU32LE(const std::uint8_t* p) { + return static_cast(p[0]) + | (static_cast(p[1]) << 8) + | (static_cast(p[2]) << 16) + | (static_cast(p[3]) << 24); +} + +std::uint64_t ReadU64LE(const std::uint8_t* p) { + return static_cast(ReadU32LE(p)) + | (static_cast(ReadU32LE(p + 4)) << 32); +} + +bool NameEquals(const std::uint8_t* fixed, std::size_t cap, const char* expected) { + // Mach-O segment/section names are NUL-padded fixed-length fields. + // We compare up to cap bytes, treating NUL as terminator on the + // `fixed` side. The expected string must not exceed cap. + const std::size_t exp_len = std::strlen(expected); + if (exp_len > cap) { + return false; + } + for (std::size_t i = 0; i < exp_len; ++i) { + if (fixed[i] != static_cast(expected[i])) { + return false; + } + } + // After the expected string, the rest must be NUL. + for (std::size_t i = exp_len; i < cap; ++i) { + if (fixed[i] != 0) { + return false; + } + } + return true; +} + +// Parses load commands of a 64-bit Mach-O whose mach_header_64 starts +// at buffer[base]. ncmds + sizeofcmds were read from the header. +// Returns the matching section or nullopt. +std::optional FindInLoadCommands64( + const std::vector& buffer, + std::size_t base, + std::uint32_t ncmds, + std::uint32_t sizeofcmds) { + // mach_header_64 is 32 bytes; load commands start right after. + constexpr std::size_t kHeader64Size = 32; + if (base + kHeader64Size > buffer.size()) { + return std::nullopt; + } + + std::size_t cursor = base + kHeader64Size; + const std::size_t end = cursor + sizeofcmds; + if (end > buffer.size()) { + return std::nullopt; + } + + for (std::uint32_t i = 0; i < ncmds; ++i) { + if (cursor + 8 > end) { + return std::nullopt; + } + const std::uint32_t cmd = ReadU32LE(buffer.data() + cursor); + const std::uint32_t cmdsize = ReadU32LE(buffer.data() + cursor + 4); + if (cmdsize < 8 || cursor + cmdsize > end) { + return std::nullopt; + } + + if (cmd == kLcSegment64) { + // segment_command_64 layout (72 bytes before sections): + // uint32 cmd, uint32 cmdsize, + // char segname[16], + // uint64 vmaddr, uint64 vmsize, + // uint64 fileoff, uint64 filesize, + // int32 maxprot, int32 initprot, + // uint32 nsects, uint32 flags + constexpr std::size_t kSeg64Size = 72; + if (cmdsize < kSeg64Size) { + return std::nullopt; + } + const std::uint8_t* segname_p = buffer.data() + cursor + 8; + const std::uint32_t nsects = ReadU32LE(buffer.data() + cursor + 64); + + if (NameEquals(segname_p, 16, kFlapiSegName)) { + // section_64 layout (80 bytes each): + // char sectname[16], + // char segname[16], + // uint64 addr, uint64 size, + // uint32 offset, uint32 align, + // uint32 reloff, uint32 nreloc, + // uint32 flags, uint32 reserved1, + // uint32 reserved2, uint32 reserved3 + constexpr std::size_t kSect64Size = 80; + std::size_t sect_cursor = cursor + kSeg64Size; + for (std::uint32_t s = 0; s < nsects; ++s) { + if (sect_cursor + kSect64Size > cursor + cmdsize) { + return std::nullopt; + } + const std::uint8_t* sn = buffer.data() + sect_cursor; + if (NameEquals(sn, 16, kFlapiSectName)) { + MachOSection out; + out.size = ReadU64LE(buffer.data() + sect_cursor + 40); + out.file_offset = ReadU32LE(buffer.data() + sect_cursor + 48); + return out; + } + sect_cursor += kSect64Size; + } + } + } else if (cmd == kLcSegment) { + // 32-bit segment. Layout differs from segment_command_64. + // uint32 cmd, uint32 cmdsize, + // char segname[16], + // uint32 vmaddr, uint32 vmsize, + // uint32 fileoff, uint32 filesize, + // int32 maxprot, int32 initprot, + // uint32 nsects, uint32 flags + constexpr std::size_t kSeg32Size = 56; + if (cmdsize < kSeg32Size) { + return std::nullopt; + } + const std::uint8_t* segname_p = buffer.data() + cursor + 8; + const std::uint32_t nsects = ReadU32LE(buffer.data() + cursor + 48); + + if (NameEquals(segname_p, 16, kFlapiSegName)) { + constexpr std::size_t kSect32Size = 68; + std::size_t sect_cursor = cursor + kSeg32Size; + for (std::uint32_t s = 0; s < nsects; ++s) { + if (sect_cursor + kSect32Size > cursor + cmdsize) { + return std::nullopt; + } + const std::uint8_t* sn = buffer.data() + sect_cursor; + if (NameEquals(sn, 16, kFlapiSectName)) { + MachOSection out; + out.size = ReadU32LE(buffer.data() + sect_cursor + 36); + out.file_offset = ReadU32LE(buffer.data() + sect_cursor + 40); + return out; + } + sect_cursor += kSect32Size; + } + } + } + + cursor += cmdsize; + } + return std::nullopt; +} + +} // namespace + +bool IsMachOMagic(const std::uint8_t magic_bytes[4]) { + const std::uint32_t m = ReadU32LE(magic_bytes); + return m == kMachOMagic32 || m == kMachOCigam32 || + m == kMachOMagic64 || m == kMachOCigam64 || + m == kFatMagic || m == kFatCigam; +} + +std::optional LocateFlapiSectionInBuffer( + const std::vector& buffer) { + if (buffer.size() < 32) { + return std::nullopt; + } + const std::uint32_t magic = ReadU32LE(buffer.data()); + + // We only handle 64-bit little-endian Mach-O here. Production + // arm64/x86_64 builds emit this format. Cigam (byte-swapped), + // 32-bit, and fat (universal) are out of scope for the spike -- + // documented in the header. + if (magic != kMachOMagic64) { + return std::nullopt; + } + + // mach_header_64: + // uint32 magic, cputype, cpusubtype, filetype, + // uint32 ncmds, sizeofcmds, flags, reserved + const std::uint32_t ncmds = ReadU32LE(buffer.data() + 16); + const std::uint32_t sizeofcmds = ReadU32LE(buffer.data() + 20); + + return FindInLoadCommands64(buffer, /*base=*/0, ncmds, sizeofcmds); +} + +std::optional LocateFlapiSection(const std::filesystem::path& path) { + std::error_code ec; + const auto file_size = std::filesystem::file_size(path, ec); + if (ec || file_size < 32) { + return std::nullopt; + } + std::ifstream in(path, std::ios::binary); + if (!in.is_open()) { + return std::nullopt; + } + // Read the header + a reasonable load-command budget (4 MiB cap so + // we don't slurp huge binaries -- normal load-cmd table is < 64 KiB). + constexpr std::size_t kReadCap = 4u * 1024u * 1024u; + const auto to_read = + static_cast(std::min(file_size, kReadCap)); + std::vector buf(to_read); + in.read(reinterpret_cast(buf.data()), + static_cast(to_read)); + if (!in) { + return std::nullopt; + } + return LocateFlapiSectionInBuffer(buf); +} + +void OverwriteFlapiSection(const std::filesystem::path& binary, + const MachOSection& section, + const std::vector& payload) { + if (payload.size() > section.size) { + throw ArchiveIOError( + "embedded archive (" + std::to_string(payload.size()) + + " bytes) exceeds reserved __FLAPI/__bundle section (" + + std::to_string(section.size) + + " bytes); rebuild flapi with a larger FLAPI_RESERVED_BUNDLE_MIB"); + } + + std::fstream out(binary, std::ios::in | std::ios::out | std::ios::binary); + if (!out.is_open()) { + throw ArchiveIOError("cannot open for in-place write: " + binary.string()); + } + + out.seekp(static_cast(section.file_offset), std::ios::beg); + if (!payload.empty()) { + out.write(reinterpret_cast(payload.data()), + static_cast(payload.size())); + } + + // Zero-pad the trailing slack so re-packs are reproducible AND so + // the EOCD reverse-scan inside the section finds the record cleanly + // without sniffing leftover bytes from a prior bundle. + const std::uint64_t slack = section.size - payload.size(); + if (slack > 0) { + std::vector zeros( + static_cast(std::min(slack, 64u * 1024u)), 0); + std::uint64_t remaining = slack; + while (remaining > 0) { + const auto chunk = + static_cast(std::min(remaining, zeros.size())); + out.write(reinterpret_cast(zeros.data()), + static_cast(chunk)); + remaining -= chunk; + } + } + + out.flush(); + if (!out) { + throw ArchiveIOError("write failure on " + binary.string()); + } +} + +CodesignResult CodesignBinary(const std::filesystem::path& binary) { + CodesignResult r; +#ifdef __APPLE__ + const char* env_id = std::getenv("CODESIGN_IDENTITY"); + r.identity = (env_id != nullptr && *env_id != '\0') ? env_id : "-"; + + // Use a popen-based invocation so we can capture stderr for the + // diagnostic tail. We pass arguments via execvp-style argv when + // we have it; for popen we shell-quote conservatively. + auto shell_quote = [](const std::string& s) { + std::string out; + out.reserve(s.size() + 2); + out.push_back('\''); + for (char c : s) { + if (c == '\'') { + out += "'\\''"; + } else { + out.push_back(c); + } + } + out.push_back('\''); + return out; + }; + + const std::string cmd = + "codesign --force --sign " + shell_quote(r.identity) + " " + + shell_quote(binary.string()) + " 2>&1"; + + FILE* pipe = popen(cmd.c_str(), "r"); + if (pipe == nullptr) { + r.exit_code = -1; + r.stderr_tail = "failed to invoke codesign (popen)"; + return r; + } + + std::array buf{}; + while (std::fgets(buf.data(), static_cast(buf.size()), pipe) != nullptr) { + r.stderr_tail.append(buf.data()); + if (r.stderr_tail.size() > 4096) { + // Keep only the tail so we don't drag a ton of output around. + r.stderr_tail.erase(0, r.stderr_tail.size() - 4096); + } + } + const int status = pclose(pipe); + if (status == -1) { + r.exit_code = -1; + } else if (WIFEXITED(status)) { + r.exit_code = WEXITSTATUS(status); + } else { + r.exit_code = -1; + } +#else + (void)binary; + r.exit_code = 0; + r.identity = "n/a"; +#endif + return r; +} + +} // namespace flapi diff --git a/src/main.cpp b/src/main.cpp index d661a8b..bba6c5b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -365,6 +365,12 @@ int main(int argc, char* argv[]) "(*.env, secrets/*, *.pem, *.key). Testing only.") .default_value(false) .implicit_value(true); + pack_cmd.add_argument("--macos-append") + .help("macOS only: append the archive after __LINKEDIT instead of " + "overwriting the reserved __FLAPI/__bundle segment. " + "Result is ad-hoc signed and NOT notarisable.") + .default_value(false) + .implicit_value(true); program.add_subparser(pack_cmd); argparse::ArgumentParser info_cmd("info"); @@ -395,6 +401,9 @@ int main(int argc, char* argv[]) try { PackOptions opts; opts.allow_secrets = pack_cmd.get("--allow-secrets"); + opts.macos_mode = pack_cmd.get("--macos-append") + ? MacOSPackMode::kAppend + : MacOSPackMode::kReservedSegment; const auto in_dir = pack_cmd.get("--in"); const auto out_path = pack_cmd.get("--out"); const auto result = Pack(in_dir, out_path, opts); diff --git a/src/pack.cpp b/src/pack.cpp index 9b1497b..61799ff 100644 --- a/src/pack.cpp +++ b/src/pack.cpp @@ -1,6 +1,7 @@ #include "pack.hpp" #include "bundle_locator.hpp" +#include "macho_bundle.hpp" #include "selfpath.hpp" #include @@ -155,17 +156,62 @@ PackResult Pack(const std::filesystem::path& in_dir, const auto archive = WriteArchive(entries, write_opts); const auto host_path = options.host_binary_override.value_or(GetSelfPath()); - const auto host_bytes = HostByteCount(host_path); - - CopyHostPrefix(host_path, out_path, host_bytes); - CopyExecutableBits(host_path, out_path); - - AppendArchive(out_path, archive); PackResult r; r.output = out_path; r.entry_count = entries.size(); r.archive_size = archive.size(); + + // Try the reserved-segment path on macOS (kReservedSegment mode). + // The Mach-O parser lives on every platform, so we can probe for + // the section anywhere -- but in practice only macOS builds emit + // one. If the host has no section, fall through to the append path. + bool used_section_path = false; + if (options.macos_mode == MacOSPackMode::kReservedSegment) { + if (auto sect = LocateFlapiSection(host_path); sect.has_value()) { + // Copy the entire host (including the placeholder section), + // then overwrite the section bytes in place. + std::error_code ec; + std::filesystem::copy_file( + host_path, out_path, + std::filesystem::copy_options::overwrite_existing, ec); + if (ec) { + throw PackError("cannot copy host binary to " + out_path.string()); + } + CopyExecutableBits(host_path, out_path); + OverwriteFlapiSection(out_path, *sect, archive); + used_section_path = true; + } + } + + if (!used_section_path) { + const auto host_bytes = HostByteCount(host_path); + CopyHostPrefix(host_path, out_path, host_bytes); + CopyExecutableBits(host_path, out_path); + AppendArchive(out_path, archive); + } + + // Re-sign on Darwin. CodesignBinary is a benign no-op on other + // platforms so we don't need a platform guard here. Note: a + // codesign failure is reported to the caller via PackError; the + // append path on macOS still benefits from an ad-hoc re-sign + // because the trailing-data invalidates the original signature. + if (options.codesign) { + auto cs = CodesignBinary(out_path); + if (cs.exit_code != 0) { + // Don't crash a Linux build over codesign output. On macOS + // a non-zero exit means we couldn't make the binary + // launchable -- surface it. +#ifdef __APPLE__ + throw PackError("codesign failed (exit " + + std::to_string(cs.exit_code) + "): " + + cs.stderr_tail); +#else + (void)cs; // unreachable in practice (we always set exit_code = 0) +#endif + } + } + return r; } diff --git a/test/cpp/CMakeLists.txt b/test/cpp/CMakeLists.txt index c271d29..add4ce1 100644 --- a/test/cpp/CMakeLists.txt +++ b/test/cpp/CMakeLists.txt @@ -23,6 +23,7 @@ add_executable(flapi_tests endpoint_config_parser_test.cpp extended_yaml_parser_test.cpp https_config_test.cpp + macho_bundle_test.cpp cache_manager_test.cpp mcp_authorization_policy_test.cpp mcp_description_scanner_test.cpp diff --git a/test/cpp/macho_bundle_test.cpp b/test/cpp/macho_bundle_test.cpp new file mode 100644 index 0000000..1b20602 --- /dev/null +++ b/test/cpp/macho_bundle_test.cpp @@ -0,0 +1,180 @@ +#include +#include "macho_bundle.hpp" + +#include +#include +#include +#include + +using namespace flapi; + +namespace { + +// Helpers to build a synthetic 64-bit little-endian Mach-O binary +// with a single LC_SEGMENT_64 containing one named section. This +// lets us unit-test the parser on any platform. + +constexpr std::uint32_t kMachOMagic64 = 0xFEEDFACFu; +constexpr std::uint32_t kLcSegment64 = 0x19; +constexpr std::size_t kHeader64Size = 32; +constexpr std::size_t kSeg64Size = 72; +constexpr std::size_t kSect64Size = 80; + +void WriteU32LE(std::vector& buf, std::size_t off, std::uint32_t v) { + buf[off] = static_cast(v & 0xff); + buf[off + 1] = static_cast((v >> 8) & 0xff); + buf[off + 2] = static_cast((v >> 16) & 0xff); + buf[off + 3] = static_cast((v >> 24) & 0xff); +} + +void WriteU64LE(std::vector& buf, std::size_t off, std::uint64_t v) { + WriteU32LE(buf, off, static_cast(v & 0xffffffffu)); + WriteU32LE(buf, off + 4, static_cast((v >> 32) & 0xffffffffu)); +} + +void WriteName(std::vector& buf, std::size_t off, + const std::string& name) { + for (std::size_t i = 0; i < 16; ++i) { + buf[off + i] = (i < name.size()) + ? static_cast(name[i]) + : 0u; + } +} + +struct SectionSpec { + std::string segname; + std::string sectname; + std::uint64_t file_offset; + std::uint64_t size; +}; + +std::vector BuildMachO64(const std::vector& sections) { + // One LC_SEGMENT_64 per section spec. Real Mach-O groups multiple + // sections under one segment, but for the parser-under-test that + // doesn't matter -- each (segname, sectname) pair is checked. + const std::uint32_t ncmds = static_cast(sections.size()); + const std::uint32_t sizeofcmds = ncmds * static_cast(kSeg64Size + kSect64Size); + + const std::size_t total = kHeader64Size + sizeofcmds; + std::vector buf(total, 0); + + // mach_header_64 + WriteU32LE(buf, 0, kMachOMagic64); // magic + WriteU32LE(buf, 4, 0x0100000Cu); // cputype = CPU_TYPE_ARM64 (arbitrary) + WriteU32LE(buf, 8, 0); // cpusubtype + WriteU32LE(buf, 12, 2); // filetype = MH_EXECUTE + WriteU32LE(buf, 16, ncmds); + WriteU32LE(buf, 20, sizeofcmds); + WriteU32LE(buf, 24, 0); // flags + WriteU32LE(buf, 28, 0); // reserved + + std::size_t cursor = kHeader64Size; + for (const auto& s : sections) { + // segment_command_64 + WriteU32LE(buf, cursor + 0, kLcSegment64); + WriteU32LE(buf, cursor + 4, static_cast(kSeg64Size + kSect64Size)); + WriteName(buf, cursor + 8, s.segname); // segname[16] + WriteU64LE(buf, cursor + 24, 0); // vmaddr + WriteU64LE(buf, cursor + 32, 0); // vmsize + WriteU64LE(buf, cursor + 40, 0); // fileoff + WriteU64LE(buf, cursor + 48, 0); // filesize + WriteU32LE(buf, cursor + 56, 0); // maxprot + WriteU32LE(buf, cursor + 60, 0); // initprot + WriteU32LE(buf, cursor + 64, 1); // nsects + WriteU32LE(buf, cursor + 68, 0); // flags + + // section_64 (immediately after the segment_command_64) + const std::size_t sect_cursor = cursor + kSeg64Size; + WriteName(buf, sect_cursor + 0, s.sectname); // sectname[16] + WriteName(buf, sect_cursor + 16, s.segname); // segname[16] + WriteU64LE(buf, sect_cursor + 32, 0); // addr + WriteU64LE(buf, sect_cursor + 40, s.size); // size + WriteU32LE(buf, sect_cursor + 48, static_cast(s.file_offset)); + WriteU32LE(buf, sect_cursor + 52, 0); // align + WriteU32LE(buf, sect_cursor + 56, 0); // reloff + WriteU32LE(buf, sect_cursor + 60, 0); // nreloc + WriteU32LE(buf, sect_cursor + 64, 0); // flags + // reserved1/2/3 already zero + + cursor += kSeg64Size + kSect64Size; + } + + return buf; +} + +} // namespace + +TEST_CASE("IsMachOMagic recognises 64-bit and fat magics", "[macho_bundle]") { + std::uint8_t mh64[4] = {0xCF, 0xFA, 0xED, 0xFE}; // little-endian 0xFEEDFACF + std::uint8_t fat[4] = {0xBE, 0xBA, 0xFE, 0xCA}; + std::uint8_t elf[4] = {0x7F, 'E', 'L', 'F'}; + + REQUIRE(IsMachOMagic(mh64)); + REQUIRE(IsMachOMagic(fat)); + REQUIRE_FALSE(IsMachOMagic(elf)); +} + +TEST_CASE("LocateFlapiSection finds __FLAPI/__bundle in synthetic Mach-O", + "[macho_bundle]") { + auto buf = BuildMachO64({ + {"__TEXT", "__text", 0x1000, 0x4000}, + {"__FLAPI", "__bundle", 0x100000, 16u * 1024u * 1024u}, + {"__DATA", "__data", 0x200000, 0x8000}, + }); + + auto loc = LocateFlapiSectionInBuffer(buf); + REQUIRE(loc.has_value()); + REQUIRE(loc->file_offset == 0x100000); + REQUIRE(loc->size == 16u * 1024u * 1024u); +} + +TEST_CASE("LocateFlapiSection returns nullopt when section is absent", + "[macho_bundle]") { + auto buf = BuildMachO64({ + {"__TEXT", "__text", 0x1000, 0x4000}, + {"__DATA", "__data", 0x10000, 0x2000}, + }); + REQUIRE_FALSE(LocateFlapiSectionInBuffer(buf).has_value()); +} + +TEST_CASE("LocateFlapiSection rejects non-Mach-O input", "[macho_bundle]") { + std::vector not_macho(256, 0); + not_macho[0] = 0x7F; // \x7FELF -- not Mach-O + not_macho[1] = 'E'; + not_macho[2] = 'L'; + not_macho[3] = 'F'; + REQUIRE_FALSE(LocateFlapiSectionInBuffer(not_macho).has_value()); +} + +TEST_CASE("LocateFlapiSection ignores a section that matches by name in the wrong segment", + "[macho_bundle]") { + // __bundle in some other segment must NOT match. + auto buf = BuildMachO64({ + {"__OTHER", "__bundle", 0x1000, 0x100}, + }); + REQUIRE_FALSE(LocateFlapiSectionInBuffer(buf).has_value()); +} + +TEST_CASE("LocateFlapiSection handles short buffer without crashing", + "[macho_bundle]") { + // mach_header_64 is 32 bytes; anything shorter must be rejected. + std::vector tiny(16, 0); + REQUIRE_FALSE(LocateFlapiSectionInBuffer(tiny).has_value()); + + // Header present but truncated load commands. + auto buf = BuildMachO64({{"__FLAPI", "__bundle", 0x1000, 0x100}}); + buf.resize(buf.size() - 32); // chop the last section_64 + REQUIRE_FALSE(LocateFlapiSectionInBuffer(buf).has_value()); +} + +TEST_CASE("CodesignBinary is a benign no-op on non-Darwin builds", + "[macho_bundle]") { +#ifndef __APPLE__ + // Path doesn't need to exist; the function shouldn't even look at it. + auto r = CodesignBinary("/nope/does/not/exist"); + REQUIRE(r.exit_code == 0); + REQUIRE(r.identity == "n/a"); +#else + SUCCEED("skipped: this test only runs on non-Darwin builds"); +#endif +} diff --git a/test/integration/test_self_packaging_macos.py b/test/integration/test_self_packaging_macos.py new file mode 100644 index 0000000..e4401a8 --- /dev/null +++ b/test/integration/test_self_packaging_macos.py @@ -0,0 +1,143 @@ +"""macOS notarised-pack tests (issue #48). + +These tests prove that the reserved __FLAPI/__bundle Mach-O segment +gets a valid signature after `flapi pack`. They're skipped on +non-Darwin runners since `codesign` doesn't exist there. +""" + +from __future__ import annotations + +import os +import pathlib +import platform +import shutil +import subprocess +import sys + +import pytest + + +sys.path.insert(0, str(pathlib.Path(__file__).parent)) +from conftest import get_flapi_binary # noqa: E402 + + +pytestmark = [ + pytest.mark.standalone_server, + pytest.mark.skipif( + platform.system() != "Darwin", + reason="macOS notarisation pack tests only run on Darwin", + ), +] + + +def _flapi() -> pathlib.Path: + return get_flapi_binary() + + +def _has_codesign() -> bool: + return shutil.which("codesign") is not None + + +def _has_otool() -> bool: + return shutil.which("otool") is not None + + +def _run(cmd, **kwargs): + return subprocess.run( + cmd, + check=False, + capture_output=True, + text=True, + timeout=180, + **kwargs, + ) + + +def _write_fixture(root: pathlib.Path) -> None: + (root / "sqls").mkdir(parents=True, exist_ok=True) + (root / "flapi.yaml").write_text( + "project-name: macos-pack-test\n" + "project-description: macOS reserved-segment fixture\n" + "template:\n path: ./sqls\n" + "connections: {}\n" + "duckdb:\n access_mode: READ_WRITE\n threads: 1\n" + ) + (root / "sqls" / "hello.yaml").write_text( + "url-path: /hello\nmethod: GET\n" + "template-source: hello.sql\nconnection: []\n" + ) + (root / "sqls" / "hello.sql").write_text("SELECT 'world' AS greeting\n") + + +def test_unbundled_flapi_has_reserved_FLAPI_bundle_segment(): + """The link-time placeholder section must exist before any pack.""" + if not _has_otool(): + pytest.skip("otool not installed") + res = _run(["otool", "-l", str(_flapi())]) + assert res.returncode == 0, res.stderr + out = res.stdout + assert "segname __FLAPI" in out, "__FLAPI segment missing from unbundled flapi" + assert "sectname __bundle" in out, "__bundle section missing from unbundled flapi" + + +def test_pack_reserved_segment_passes_codesign_verify(tmp_path: pathlib.Path): + """Default pack on macOS overwrites the segment and re-signs.""" + if not _has_codesign(): + pytest.skip("codesign not installed") + + fixture = tmp_path / "fixture" + _write_fixture(fixture) + out = tmp_path / "flapi-bundled" + + res = _run([str(_flapi()), "pack", "--in", str(fixture), "--out", str(out)]) + assert res.returncode == 0, f"pack failed: {res.stderr}" + assert out.exists() + + verify = _run(["codesign", "--verify", "--strict", str(out)]) + assert verify.returncode == 0, ( + f"codesign --verify failed on reserved-segment output:\n" + f"stdout={verify.stdout}\nstderr={verify.stderr}" + ) + + +def test_macos_append_legacy_path_still_packs(tmp_path: pathlib.Path): + """`--macos-append` writes the legacy trailing-bytes layout. The + result is NOT notarisable, but pack must still produce a runnable + artifact with a discoverable bundle.""" + fixture = tmp_path / "fixture" + _write_fixture(fixture) + out = tmp_path / "flapi-bundled-append" + + res = _run([ + str(_flapi()), "pack", + "--in", str(fixture), "--out", str(out), + "--macos-append", + ]) + assert res.returncode == 0, f"pack --macos-append failed: {res.stderr}" + + info = _run([str(out), "info"]) + assert info.returncode == 0 + assert "flapi.yaml" in info.stdout + + +def test_oversized_payload_is_rejected_clearly(tmp_path: pathlib.Path): + """If the archive doesn't fit in the reserved segment, pack must + refuse with a corrective message rather than corrupt the binary.""" + fixture = tmp_path / "fixture" + _write_fixture(fixture) + + # Default reserved size is 16 MiB. Stuff in a ~32 MiB blob so the + # archive can't possibly fit. + big = fixture / "data" / "huge.bin" + big.parent.mkdir(parents=True, exist_ok=True) + with big.open("wb") as f: + f.write(os.urandom(32 * 1024 * 1024)) + + out = tmp_path / "flapi-bundled-too-big" + res = _run([str(_flapi()), "pack", "--in", str(fixture), "--out", str(out)]) + assert res.returncode != 0 + combined = (res.stdout + res.stderr).lower() + assert "reserved" in combined or "exceeds" in combined, combined + assert "flapi_reserved_bundle_mib" in combined.replace("-", "_"), ( + "error message should mention the rebuild knob" + ) From b514ea0705fb9255bbeaf4ecaa3702a7b9b517b8 Mon Sep 17 00:00:00 2001 From: Joachim Rosskopf Date: Fri, 22 May 2026 19:52:58 +0200 Subject: [PATCH 2/2] fix(cmake): bypass shell wrapper for macOS placeholder generation The previous `cmake -E env bash -c "dd ... >/dev/null 2>&1"` form broke under ninja-on-macos -- the redirect operators were consumed by the outer shell that cmake exec'd, not the bash -c subshell, so the build failed with: /bin/sh: /dev/null 2: Permission denied Drop the shell wrapper entirely. dd's argv form (`if=...`, `of=...`, `bs=1m`, `count=N`) goes straight to execve via CMake's command runner, no shell involved. dd's 2-3 line summary stays in CI logs, which is fine. Found by CI on PR #58 (#48 macOS notarised) at osx-universal-build. --- CMakeLists.txt | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f975a39..1f2912c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -355,10 +355,18 @@ if(APPLE) "Size in MiB of the reserved __FLAPI/__bundle Mach-O segment.") set(FLAPI_BUNDLE_PLACEHOLDER "${CMAKE_BINARY_DIR}/_flapi_bundle_placeholder.bin") + # Use dd directly via execve (no shell). The previous `bash -c + # "... >/dev/null 2>&1"` wrapping broke under ninja-on-macos because + # the redirect operators were parsed by the outer shell rather than + # the bash -c subshell, producing "/dev/null 2: Permission denied". + # dd prints a 2-3 line summary on stderr; that's fine in CI logs. add_custom_command( OUTPUT "${FLAPI_BUNDLE_PLACEHOLDER}" - COMMAND ${CMAKE_COMMAND} -E env - bash -c "dd if=/dev/zero of='${FLAPI_BUNDLE_PLACEHOLDER}' bs=1m count=${FLAPI_RESERVED_BUNDLE_MIB} >/dev/null 2>&1" + COMMAND dd + if=/dev/zero + "of=${FLAPI_BUNDLE_PLACEHOLDER}" + bs=1m + count=${FLAPI_RESERVED_BUNDLE_MIB} COMMENT "flapi: generating ${FLAPI_RESERVED_BUNDLE_MIB}-MiB __FLAPI/__bundle placeholder") add_custom_target(flapi_bundle_placeholder DEPENDS "${FLAPI_BUNDLE_PLACEHOLDER}")