From 63780b2461282aa9ad9ad655bf96b050bbadad67 Mon Sep 17 00:00:00 2001 From: Khushboo Mehta Date: Fri, 7 Aug 2026 15:37:10 +0200 Subject: [PATCH] =?UTF-8?q?feat:=20manifest=200.4.0=20=E2=80=94=20variant-?= =?UTF-8?q?independent=20icon=20in=20assets/?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `icon` moves from a per-variant copy to a single assets/icon.png at the package root, so a host can read it without unpacking a platform build — the precondition for showing icons before install. Covered by the Merkle tree, so it is authenticated by manifest.sig for free. Adds `lgx add --icon` and Package::setIcon(). Validation (PNG, exactly 256x256) runs at verify/sign, not at create/add — a package is legitimately incomplete mid-assembly. Required for type ui_qml only; core modules render no tile. Manifests below 0.4.0 are exempt: icon: "" was legal at 0.3.0 and is what lgx create defaulted to, so enforcing it would make every published package uninstallable --- docs/spec.md | 54 +++++- doctests/lgx-cli.test.yaml | 4 +- doctests/outputs/lgx-cli.md | 4 +- doctests/outputs/lgx-signing.md | 2 +- src/commands/add_command.cpp | 31 +++- src/commands/add_command.h | 4 + src/core/manifest.cpp | 28 +++- src/core/manifest.h | 23 ++- src/core/package.cpp | 165 +++++++++++++++++- src/core/package.h | 26 +++ tests/test_cli.cpp | 22 ++- tests/test_package.cpp | 285 ++++++++++++++++++++++++++++++++ tests/test_png.h | 84 ++++++++++ 13 files changed, 709 insertions(+), 23 deletions(-) create mode 100644 tests/test_png.h diff --git a/docs/spec.md b/docs/spec.md index 7da1cac..3effed4 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -34,6 +34,8 @@ An LGX package (`.lgx` file) is a gzip-compressed tar archive with the following package.lgx (tar.gz) ├── manifest.json # Required - package metadata ├── manifest.sig # Optional - Ed25519 signature with DID identity +├── assets/ # Optional - variant-independent package assets +│ └── icon.png # Package icon: PNG, exactly 256x256 ├── variants/ # Required - contains variant directories │ ├── / # Variant directory (lowercase name) │ │ └── ... # Variant contents @@ -46,25 +48,25 @@ package.lgx (tar.gz) ``` **Root Entry Constraints:** -- Only `manifest.json`, `manifest.cose`, `variants/`, `docs/`, and `licenses/` are permitted at root +- Only `manifest.json`, `manifest.sig`, `assets/`, `variants/`, `docs/`, and `licenses/` are permitted at root - Any other root entries cause validation failure - Files directly under `variants/` are forbidden (only directories allowed) - This strict structure keeps packages easy to validate and reduces ambiguity ### Manifest Schema -The current manifest schema is `0.3.0`. It is a UTF-8 encoded JSON file with the following required fields: +The current manifest schema is `0.4.0`. It is a UTF-8 encoded JSON file with the following required fields: ```json { - "manifestVersion": "0.3.0", + "manifestVersion": "0.4.0", "name": "package-name", "version": "1.2.3", "description": "Package description", "author": "Author Name", "type": "library", "category": "crypto", - "icon": "icon.png", + "icon": "assets/icon.png", "dependencies": [ "simple-dep", {"name": "ranged-dep", "version": "^1.2.0"}, @@ -88,7 +90,7 @@ The current manifest schema is `0.3.0`. It is a UTF-8 encoded JSON file with the | `author` | string | Author/maintainer name | Human metadata | | `type` | string | Package type classification | Classification | | `category` | string | Package category | Classification | -| `icon` | string | Relative path to icon file bundled in the package | Display/branding | +| `icon` | string | Relative path to the icon bundled in the package. At `0.4.0`+ this is `assets/icon.png` — see *Icon contract* below | Display/branding | | `dependencies` | array | List of dependency entries — see *Dependency entries* below | Runtime needs | | `main` | object | Map of variant name → relative path to entry point (e.g ) `"linux-amd64": "path/to/main.so"` means `linux-amd64/path/to/main.so` | Entry point resolution | | `display_name` | string | *Optional.* Human-readable label shown by UI consumers (Package Manager, App Manager) and CLI tools (`lm metadata`, `lgx manifest`). Falls back to `name` when absent. | Display/branding | @@ -111,6 +113,48 @@ Each element of the `dependencies` array is one of: Tooling reads both `manifestVersion: "0.2.x"` and `manifestVersion: "0.3.x"`. Packages produced by `lgx create` use `0.3.0`. A 0.2.0 manifest with plain-string dependencies round-trips unchanged through tooling — strings are emitted as strings, object-form entries are emitted as objects. Bumping the major version (1.x.x) is reserved for future breaking changes. +### Icon Contract + +At `manifestVersion` `0.4.0`+, `icon` points at a **root-level, variant-independent +asset**: `assets/icon.png`. + +**Requirements** + +| Rule | Value | +|------|-------| +| Format | PNG | +| Dimensions | **exactly** 256x256 — not a minimum | +| Location | `assets/icon.png` (canonical; set by `lgx add --icon`) | +| Required for | `type == "ui_qml"` | +| Optional for | every other type, including `core` | + +**Why root-level rather than per-variant.** One copy instead of one per +platform, and a host can read the icon without choosing or unpacking a +platform build — which is what lets a registry extract it at publish time and +serve it to clients that have not downloaded the package. `assets/` is inside +the Merkle tree, so the icon is authenticated by `manifest.sig` for free. + +**Why exactly 256x256.** The artifact stays byte-predictable: the icon in +`assets/` is the author's file unmodified, so the content hash is reproducible +from source without depending on an image library's resampling behaviour. It +also keeps the packaging step free of an image-processing dependency — +dimension validation is a fixed-offset PNG `IHDR` read. + +**Why `ui_qml` only.** UI packages render a tile in the App Manager, sidebar +and launcher. Core modules appear in package lists but have no such surface, +so requiring artwork from them would be cost without benefit. + +**Validation timing.** `lgx create` and `lgx add` are construction steps and do +not enforce the contract — a package may be assembled in any order. Enforcement +happens at `lgx verify`, `lgx sign` and `lgpm install`, i.e. wherever a package +is asserted to be complete. + +**Backward compatibility.** Manifests below `0.4.0` are **exempt**. `icon: ""` +was legal at `0.3.0` (it is what `lgx create` defaulted to), so applying the +rule unconditionally would make every already-published package fail +verification and become uninstallable. Tooling reads `0.2.x`, `0.3.x` and +`0.4.x`; only `0.4.0`+ carries the icon contract. + ### `ui_qml` Contract For most package types, `main` remains the per-variant entry point and is required. diff --git a/doctests/lgx-cli.test.yaml b/doctests/lgx-cli.test.yaml index c24ea1f..d1b8977 100644 --- a/doctests/lgx-cli.test.yaml +++ b/doctests/lgx-cli.test.yaml @@ -104,7 +104,7 @@ sections: - title: "Read the fresh manifest" text: | `lgx manifest` prints the embedded `manifest.json` in human-readable form. A - freshly created package starts at version `0.0.1`, manifest schema `0.3.0`, with + freshly created package starts at version `0.0.1`, manifest schema `0.4.0`, with no type, no variants, and no signature yet. run: "./lgx/bin/lgx manifest greeter.lgx" code_block: "lgx manifest greeter.lgx" @@ -112,7 +112,7 @@ sections: - "greeter" - "0.0.1" - "Manifest ver.:" - - "0.3.0" + - "0.4.0" - "Variants: (none)" - "Signed: no" diff --git a/doctests/outputs/lgx-cli.md b/doctests/outputs/lgx-cli.md index 2de8651..065fd68 100644 --- a/doctests/outputs/lgx-cli.md +++ b/doctests/outputs/lgx-cli.md @@ -57,7 +57,7 @@ link the result as `./lgx`, so the binary lands at `./lgx/bin/lgx`. ```bash # From inside the clone this is simply: nix build '.#lgx' -o lgx -nix build 'github:logos-co/logos-package/41cae9e4546899eb11912372588a6707ca43efe4#lgx' -o lgx +nix build 'github:logos-co/logos-package/43378dab2ffc569a28f144d3f62a0f089c3580df#lgx' -o lgx ``` The `-o lgx` flag names the result symlink, so the executable is at `./lgx/bin/lgx`. @@ -108,7 +108,7 @@ sensible defaults plus an empty `variants/` directory — to `.lgx`. ### 3.2 Read the fresh manifest `lgx manifest` prints the embedded `manifest.json` in human-readable form. A -freshly created package starts at version `0.0.1`, manifest schema `0.3.0`, with +freshly created package starts at version `0.0.1`, manifest schema `0.4.0`, with no type, no variants, and no signature yet. ```bash diff --git a/doctests/outputs/lgx-signing.md b/doctests/outputs/lgx-signing.md index aeb94af..2548779 100644 --- a/doctests/outputs/lgx-signing.md +++ b/doctests/outputs/lgx-signing.md @@ -55,7 +55,7 @@ detail; here they are just setup.) ```bash # From inside the clone this is simply: nix build '.#lgx' -o lgx -nix build 'github:logos-co/logos-package/41cae9e4546899eb11912372588a6707ca43efe4#lgx' -o lgx +nix build 'github:logos-co/logos-package/43378dab2ffc569a28f144d3f62a0f089c3580df#lgx' -o lgx ``` ### 1.2 Create and populate greeter.lgx diff --git a/src/commands/add_command.cpp b/src/commands/add_command.cpp index e9b3389..300f6ab 100644 --- a/src/commands/add_command.cpp +++ b/src/commands/add_command.cpp @@ -3,6 +3,9 @@ #include "core/path_normalizer.h" #include +#include +#include +#include namespace lgx { @@ -34,6 +37,7 @@ int AddCommand::execute(const std::vector& args) { std::string mainPath = getOption(opts, "main", "m"); std::string viewPath = getOption(opts, "view"); + std::string iconPath = getOption(opts, "icon"); bool autoYes = hasFlag(opts, "yes", "y"); // Check if package exists @@ -67,7 +71,32 @@ int AddCommand::execute(const std::vector& args) { } pkg.getManifest().view = viewPath; } - + + // Apply --icon: stage the file at the canonical assets/ path and point the + // manifest at it. Variant-independent by design — one icon serves every + // platform build, so this is set on the package rather than per variant. + // Validation of the bytes (PNG, exactly 256x256) happens in + // Package::validateIconAsset() when the package is verified or signed. + if (!iconPath.empty()) { + if (!std::filesystem::exists(iconPath)) { + printError("Icon not found: " + iconPath); + return 1; + } + std::ifstream iconFile(iconPath, std::ios::binary); + if (!iconFile) { + printError("Cannot read icon: " + iconPath); + return 1; + } + std::vector iconData( + (std::istreambuf_iterator(iconFile)), + std::istreambuf_iterator()); + auto iconResult = pkg.setIcon(iconData); + if (!iconResult.success) { + printError("Failed to set icon: " + iconResult.error); + return 1; + } + } + // Check if variant exists (replacement warning) bool variantExists = pkg.hasVariant(variantLc); diff --git a/src/commands/add_command.h b/src/commands/add_command.h index fc6dfbb..cc67580 100644 --- a/src/commands/add_command.h +++ b/src/commands/add_command.h @@ -32,6 +32,10 @@ public: " --view QML entry point relative to variant root\n" " (required for `ui_qml` packages; sets the\n" " manifest-level `view` field)\n" + " --icon Package icon: PNG, exactly 256x256. Stored\n" + " once at assets/icon.png and shared by every\n" + " variant. Required for `ui_qml` packages at\n" + " manifest version 0.4.0+\n" " --yes, -y Skip confirmation prompts\n" "\n" "Examples:\n" diff --git a/src/core/manifest.cpp b/src/core/manifest.cpp index 1344af1..ab07a8a 100644 --- a/src/core/manifest.cpp +++ b/src/core/manifest.cpp @@ -412,12 +412,34 @@ bool Manifest::isVersionSupported(const std::string& version) { std::string major = version.substr(0, dotPos); // Currently only major version 0 is supported. Within 0.x we accept - // both 0.2.* (legacy plain-string dependencies) and 0.3.* (richer - // dependencies with optional version range + signer DID); see the - // Dependency parsing in fromJson() for the compatibility shim. + // 0.2.* (legacy plain-string dependencies), 0.3.* (richer dependencies + // with optional version range + signer DID) and 0.4.* (root-level + // assets/ slot); see the Dependency parsing in fromJson() for the + // compatibility shim and requiresIconContract() for the icon gate. return major == "0"; } +bool Manifest::requiresIconContract(const std::string& version) { + // Parse ".." — anything below 0.4 predates assets/. + const size_t firstDot = version.find('.'); + if (firstDot == std::string::npos) return false; + const size_t secondDot = version.find('.', firstDot + 1); + + const std::string majorStr = version.substr(0, firstDot); + const std::string minorStr = secondDot == std::string::npos + ? version.substr(firstDot + 1) + : version.substr(firstDot + 1, secondDot - firstDot - 1); + + try { + const int major = std::stoi(majorStr); + const int minor = std::stoi(minorStr); + if (major > 0) return true; // 1.x and beyond + return major == 0 && minor >= 4; // 0.4.0+ + } catch (const std::exception&) { + return false; // unparseable → treat as legacy + } +} + Manifest::ValidationResult Manifest::compareMetadata(const Manifest& other) const { ValidationResult result = ValidationResult::ok(); diff --git a/src/core/manifest.h b/src/core/manifest.h index efe78e9..8ebf65a 100644 --- a/src/core/manifest.h +++ b/src/core/manifest.h @@ -56,10 +56,25 @@ struct Dependency { */ class Manifest { public: - // Current manifest version. Bumped to 0.3.0 to accommodate richer - // dependency entries (semver ranges + optional signer DID). 0.2.x - // manifests are still readable for backward compatibility. - static constexpr const char* CURRENT_VERSION = "0.3.0"; + // Current manifest version. Bumped to 0.4.0 for the root-level assets/ + // slot: `icon` now points at a variant-independent asset (assets/icon.png) + // that is exactly 256x256 PNG, so hosts can display it without unpacking + // a platform build. 0.2.x and 0.3.x manifests are still readable. + static constexpr const char* CURRENT_VERSION = "0.4.0"; + + // Canonical in-package icon location for 0.4.0+. The author's + // metadata.json path stays free-form; the bundler normalises to this. + static constexpr const char* ICON_PATH = "assets/icon.png"; + + // Required icon dimensions, enforced for 0.4.0+ packages whose type + // requires an icon. Exact, not a minimum — see plan.md §3.4. + static constexpr int ICON_SIZE_PX = 256; + + // True when `version` is 0.4.0 or newer, i.e. the icon contract applies. + // 0.2.x/0.3.x packages predate it and are exempt (plan.md §3.7): `icon` + // was legal as "" back then, so applying the rule unconditionally would + // make every already-published package uninstallable. + static bool requiresIconContract(const std::string& version); /** * Validation result for manifest. diff --git a/src/core/package.cpp b/src/core/package.cpp index 51a2d82..4ee00c9 100644 --- a/src/core/package.cpp +++ b/src/core/package.cpp @@ -10,10 +10,121 @@ namespace lgx { thread_local std::string Package::lastError_; +namespace { + +// PNG header layout is fixed-offset, so dimensions come out of the first 26 +// bytes with no image library: 8-byte signature, 4-byte IHDR length, 4-byte +// "IHDR" tag, then width and height as big-endian uint32. +// +// This validates the *declared* dimensions only. It is deliberately not a +// defence against a malicious payload — a PNG can claim 256x256 and carry an +// enormous IDAT. Byte-size ceilings and decoder allocation limits belong at +// the fetch/decode boundary (plan.md §3.2.2), not here. +struct PngHeader { + bool valid = false; + uint32_t width = 0; + uint32_t height = 0; +}; + +PngHeader readPngHeader(const std::vector& data) { + static const uint8_t kSignature[8] = + {0x89, 'P', 'N', 'G', '\r', '\n', 0x1A, '\n'}; + + PngHeader h; + if (data.size() < 26) return h; + if (!std::equal(std::begin(kSignature), std::end(kSignature), data.begin())) + return h; + if (data[12] != 'I' || data[13] != 'H' || + data[14] != 'D' || data[15] != 'R') return h; + + auto be32 = [&data](size_t off) { + return (static_cast(data[off]) << 24) + | (static_cast(data[off + 1]) << 16) + | (static_cast(data[off + 2]) << 8) + | static_cast(data[off + 3]); + }; + + h.width = be32(16); + h.height = be32(20); + h.valid = true; + return h; +} + +} // namespace + +void Package::validateIconAsset(VerifyResult& result) const { + // Version gate. 0.2.x/0.3.x packages predate the assets/ slot and were + // free to carry `icon: ""`; enforcing the contract on them would render + // the entire already-published catalog uninstallable. See plan.md §3.7. + if (!Manifest::requiresIconContract(manifest_.manifestVersion)) return; + + // Icons are required for UI packages, which are the ones that render a + // tile. Core modules appear in package lists but have no launcher or + // sidebar presence, so an icon stays optional for them. + const bool iconRequired = (manifest_.type == "ui_qml"); + + if (manifest_.icon.empty()) { + if (iconRequired) { + result.valid = false; + result.errors.push_back( + std::string("Missing 'icon': ") + Manifest::ICON_PATH + + " is required for type '" + manifest_.type + "' packages"); + } + return; + } + + // The canonical location is part of the contract, not a convention: a + // host resolves /assets/icon.png without consulting the + // manifest, and the release tool globs for it. Allowing `icon` to point + // anywhere would make both unpredictable. + if (manifest_.icon != Manifest::ICON_PATH) { + result.valid = false; + result.errors.push_back( + std::string("Manifest 'icon' must be '") + Manifest::ICON_PATH + + "' at manifestVersion 0.4.0+, got '" + manifest_.icon + "'"); + return; + } + + const TarEntry* iconEntry = nullptr; + for (const auto& entry : entries_) { + if (entry.path == manifest_.icon && !entry.isDirectory) { + iconEntry = &entry; + break; + } + } + + if (!iconEntry) { + result.valid = false; + result.errors.push_back( + "Manifest 'icon' points at '" + manifest_.icon + + "' which is not present in the package"); + return; + } + + const PngHeader header = readPngHeader(iconEntry->data); + if (!header.valid) { + result.valid = false; + result.errors.push_back( + "Icon '" + manifest_.icon + "' does not match the Logos icon " + "standard: expected PNG, exactly 256x256; actual: not a PNG"); + return; + } + + const uint32_t want = static_cast(Manifest::ICON_SIZE_PX); + if (header.width != want || header.height != want) { + result.valid = false; + result.errors.push_back( + "Icon '" + manifest_.icon + "' does not match the Logos icon " + "standard: expected PNG, exactly 256x256; actual: PNG, " + + std::to_string(header.width) + "x" + std::to_string(header.height)); + } +} + const std::set Package::ALLOWED_ROOT_ENTRIES = { "manifest.json", "manifest.sig", "variants", + "assets", "docs", "licenses" }; @@ -246,6 +357,8 @@ Package::VerifyResult Package::validatePackage() const { result.errors.push_back("Missing variants/ directory"); } + validateIconAsset(result); + // Validate completeness (variants <-> main mapping) auto completenessResult = manifest_.validateCompleteness(foundVariants); if (!completenessResult.valid) { @@ -656,8 +769,20 @@ Package::Result Package::extractVariant( std::string prefix = "variants/" + variantLc + "/"; + // Root-level `assets/` is variant-independent and must land in the SAME + // output directory as the variant contents, because the manifest's `icon` + // is documented as relative to the installed package root. Extracting + // only `variants//` left `assets/icon.png` on the floor, so every + // installed 0.4.0 package resolved its icon to a missing file and fell + // back to the monogram — a regression against 0.3.x, where the icon lived + // inside the variant and therefore did extract. + const std::string assetsPrefix = "assets/"; + for (const auto& entry : entries_) { - if (entry.path.substr(0, prefix.length()) != prefix) { + const bool inVariant = entry.path.compare(0, prefix.length(), prefix) == 0; + const bool inAssets = + entry.path.compare(0, assetsPrefix.length(), assetsPrefix) == 0; + if (!inVariant && !inAssets) { continue; } @@ -673,7 +798,10 @@ Package::Result Package::extractVariant( entry.path + "': " + pathValidation.error); } - std::string relativePath = entry.path.substr(prefix.length()); + // Variant entries are rebased to the variant root; asset entries keep + // their `assets/...` path so the installed layout matches the manifest. + std::string relativePath = + inVariant ? entry.path.substr(prefix.length()) : entry.path; if (relativePath.empty()) { continue; } @@ -851,6 +979,39 @@ void Package::clearSignature() { manifestSig_ = std::nullopt; } +Package::Result Package::setIcon(const std::vector& pngData) { + if (pngData.empty()) { + return Result::fail("Icon data is empty"); + } + + const std::string iconPath = Manifest::ICON_PATH; + + // Replace any existing entry at the canonical path. + entries_.erase( + std::remove_if(entries_.begin(), entries_.end(), + [&iconPath](const TarEntry& e) { + return e.path == iconPath; + }), + entries_.end()); + + TarEntry iconEntry; + iconEntry.path = iconPath; + iconEntry.data = pngData; + iconEntry.isDirectory = false; + entries_.push_back(std::move(iconEntry)); + + manifest_.icon = iconPath; + + // Invalidate signature and recompute hashes (content changed) + clearSignature(); + auto hashResult = recomputeHashes(); + if (!hashResult.success) { + return hashResult; + } + + return Result::ok(); +} + Package::Result Package::recomputeHashes() { if (!crypto::init()) { return Result::fail("Failed to initialize crypto library — cannot compute content hashes"); diff --git a/src/core/package.h b/src/core/package.h index 4c92b81..5c3b3c8 100644 --- a/src/core/package.h +++ b/src/core/package.h @@ -83,6 +83,22 @@ public: */ static VerifyResult verify(const std::filesystem::path& lgxPath); + /** + * Set (or replace) the package icon. + * + * Writes the bytes to the canonical assets/icon.png entry and points the + * manifest's `icon` field at it. The icon is variant-independent: one + * copy serves every platform build, and a host can read it without + * choosing or unpacking a variant. + * + * Byte validation (PNG, exactly 256x256) is deliberately NOT done here — + * it happens in validateIconAsset() at verify/sign time, so a package can + * be assembled in any order. + * + * Invalidates any existing signature and recomputes content hashes. + */ + Result setIcon(const std::vector& pngData); + /** * Add files to a variant. * If the variant exists, it is completely replaced. @@ -232,6 +248,16 @@ private: static thread_local std::string lastError_; + /** + * Validate the packaged icon asset against the Logos icon standard + * (PNG, exactly 256x256, at the manifest's `icon` path). + * + * No-op for manifests older than 0.4.0 — they predate the assets/ slot + * and legitimately carry `icon: ""`, so enforcing would break every + * already-published package. See Manifest::requiresIconContract(). + */ + void validateIconAsset(VerifyResult& result) const; + /** * Rebuild tar entries, ensuring manifest is included. */ diff --git a/tests/test_cli.cpp b/tests/test_cli.cpp index d966f96..0178566 100644 --- a/tests/test_cli.cpp +++ b/tests/test_cli.cpp @@ -3,6 +3,8 @@ #include #include #include + +#include "test_png.h" #include namespace fs = std::filesystem; @@ -329,9 +331,14 @@ TEST_F(CLITest, AddCommand_UiQmlDirectoryWithoutMain) { auto saveResult = pkgOpt->save(pkgPath); ASSERT_TRUE(saveResult.success); + // 0.4.0 requires ui_qml packages to carry a conforming icon. + const std::string iconPath = + lgx_test::writePng((tempDir / "icon.png").string()); + std::string output; int exitCode = runLgx( - "add " + pkgPath.string() + " -v darwin-arm64 -f " + testDir.string() + " -y", + "add " + pkgPath.string() + " -v darwin-arm64 -f " + testDir.string() + + " --icon " + iconPath + " -y", &output ); @@ -356,10 +363,14 @@ TEST_F(CLITest, AddCommand_UiQmlDirectoryWithViewFlag) { auto saveResult = pkgOpt->save(pkgPath); ASSERT_TRUE(saveResult.success); + // 0.4.0 requires ui_qml packages to carry a conforming icon. + const std::string iconPath = + lgx_test::writePng((tempDir / "icon.png").string()); + std::string output; int exitCode = runLgx( "add " + pkgPath.string() + " -v darwin-arm64 -f " + testDir.string() - + " --view qml/Main.qml -y", + + " --view qml/Main.qml --icon " + iconPath + " -y", &output ); @@ -388,9 +399,14 @@ TEST_F(CLITest, AddCommand_UiQmlDirectoryWithoutView) { auto saveResult = pkgOpt->save(pkgPath); ASSERT_TRUE(saveResult.success); + // 0.4.0 requires ui_qml packages to carry a conforming icon. + const std::string iconPath = + lgx_test::writePng((tempDir / "icon.png").string()); + std::string output; int exitCode = runLgx( - "add " + pkgPath.string() + " -v darwin-arm64 -f " + testDir.string() + " -y", + "add " + pkgPath.string() + " -v darwin-arm64 -f " + testDir.string() + + " --icon " + iconPath + " -y", &output ); diff --git a/tests/test_package.cpp b/tests/test_package.cpp index 98b939a..f70c765 100644 --- a/tests/test_package.cpp +++ b/tests/test_package.cpp @@ -8,6 +8,9 @@ #include #include +#include + +#include "test_png.h" using namespace lgx; namespace fs = std::filesystem; @@ -234,6 +237,8 @@ TEST_F(PackageTest, AddVariant_UiQmlDirectory_AllowsMissingMain) { ASSERT_TRUE(pkg.has_value()); pkg->getManifest().type = "ui_qml"; pkg->getManifest().view = "qml/Main.qml"; + // 0.4.0 requires ui_qml packages to carry a conforming icon. + ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success); fs::path testDir = tempDir / "dist"; createTestDirectory(testDir, { @@ -262,6 +267,8 @@ TEST_F(PackageTest, AddVariant_UiQmlDirectory_ClearsStaleMain) { ASSERT_TRUE(pkg.has_value()); pkg->getManifest().type = "ui_qml"; pkg->getManifest().view = "qml/Main.qml"; + // 0.4.0 requires ui_qml packages to carry a conforming icon. + ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success); fs::path firstDir = tempDir / "first"; createTestDirectory(firstDir, { @@ -276,6 +283,8 @@ TEST_F(PackageTest, AddVariant_UiQmlDirectory_ClearsStaleMain) { ASSERT_TRUE(pkg.has_value()); pkg->getManifest().type = "ui_qml"; pkg->getManifest().view = "qml/Main.qml"; + // 0.4.0 requires ui_qml packages to carry a conforming icon. + ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success); fs::path secondDir = tempDir / "second"; createTestDirectory(secondDir, { @@ -1310,3 +1319,279 @@ TEST_F(PackageTest, Verify_SignedPackage_ValidHashes) { EXPECT_TRUE(result.valid) << "Errors: " << (result.errors.empty() ? "none" : result.errors[0]); } + +// ============================================================================= +// Icon Contract (manifest 0.4.0+) — plan.md §3.4, §3.7 +// ============================================================================= + +TEST_F(PackageTest, Icon_ConformingIconValidates) { + fs::path pkgPath = tempDir / "test.lgx"; + Package::create(pkgPath, "testpkg"); + + auto pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + pkg->getManifest().type = "ui_qml"; + pkg->getManifest().view = "qml/Main.qml"; + ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success); + + fs::path testDir = tempDir / "dist"; + createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}}); + ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success); + ASSERT_TRUE(pkg->save(pkgPath).success); + + auto result = Package::verify(pkgPath); + EXPECT_TRUE(result.valid) << "Errors: " << + (result.errors.empty() ? "none" : result.errors[0]); +} + +// The icon lands at the canonical root path, NOT inside a variant — that is +// what makes it readable without unpacking a platform build. +TEST_F(PackageTest, Icon_StoredAtCanonicalRootPath) { + fs::path pkgPath = tempDir / "test.lgx"; + Package::create(pkgPath, "testpkg"); + + auto pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success); + ASSERT_TRUE(pkg->save(pkgPath).success); + + pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + EXPECT_EQ(pkg->getManifest().icon, std::string("assets/icon.png")); + + bool found = false; + for (const auto& e : pkg->getEntries()) + if (e.path == "assets/icon.png" && !e.isDirectory) found = true; + EXPECT_TRUE(found) << "icon must live at assets/icon.png"; +} + +TEST_F(PackageTest, Icon_WrongDimensionsRejected) { + fs::path pkgPath = tempDir / "test.lgx"; + Package::create(pkgPath, "testpkg"); + + auto pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + pkg->getManifest().type = "ui_qml"; + pkg->getManifest().view = "qml/Main.qml"; + ASSERT_TRUE(pkg->setIcon(lgx_test::makePng(512, 512)).success); + + fs::path testDir = tempDir / "dist"; + createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}}); + ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success); + pkg->save(pkgPath); + + auto result = Package::verify(pkgPath); + EXPECT_FALSE(result.valid); + ASSERT_FALSE(result.errors.empty()); + // The message must name the standard and both sizes, not just assert. + EXPECT_NE(result.errors[0].find("256x256"), std::string::npos); + EXPECT_NE(result.errors[0].find("512x512"), std::string::npos); +} + +TEST_F(PackageTest, Icon_NonPngRejected) { + fs::path pkgPath = tempDir / "test.lgx"; + Package::create(pkgPath, "testpkg"); + + auto pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + pkg->getManifest().type = "ui_qml"; + pkg->getManifest().view = "qml/Main.qml"; + std::vector notPng(64, 0x41); // "AAAA..." + ASSERT_TRUE(pkg->setIcon(notPng).success); + + fs::path testDir = tempDir / "dist"; + createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}}); + ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success); + pkg->save(pkgPath); + + auto result = Package::verify(pkgPath); + EXPECT_FALSE(result.valid); + ASSERT_FALSE(result.errors.empty()); + EXPECT_NE(result.errors[0].find("not a PNG"), std::string::npos); +} + +TEST_F(PackageTest, Icon_MissingOnUiQmlRejected) { + fs::path pkgPath = tempDir / "test.lgx"; + Package::create(pkgPath, "testpkg"); + + auto pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + pkg->getManifest().type = "ui_qml"; + pkg->getManifest().view = "qml/Main.qml"; + + fs::path testDir = tempDir / "dist"; + createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}}); + ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success); + pkg->save(pkgPath); + + auto result = Package::verify(pkgPath); + EXPECT_FALSE(result.valid); +} + +// Core modules appear in package lists but render no tile, so an icon stays +// optional for them. Requiring one would turn a 12-module migration into a +// 33-module one for no visual benefit. +TEST_F(PackageTest, Icon_OptionalForCoreType) { + fs::path pkgPath = tempDir / "test.lgx"; + Package::create(pkgPath, "testpkg"); + + auto pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + pkg->getManifest().type = "core"; + + fs::path testFile = tempDir / "mod.so"; + createTestFile(testFile, "binary"); + ASSERT_TRUE(pkg->addVariant("linux-amd64", testFile).success); + ASSERT_TRUE(pkg->save(pkgPath).success); + + auto result = Package::verify(pkgPath); + EXPECT_TRUE(result.valid) << "Errors: " << + (result.errors.empty() ? "none" : result.errors[0]); +} + +// THE backward-compatibility regression. A 0.3.0 ui_qml package with icon:"" +// was legal and is published in the wild. Enforcing the 0.4.0 contract on it +// would make the entire existing catalog fail verification and become +// uninstallable. See plan.md §3.7. +TEST_F(PackageTest, Icon_LegacyManifestExemptFromContract) { + fs::path pkgPath = tempDir / "test.lgx"; + Package::create(pkgPath, "testpkg"); + + auto pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + pkg->getManifest().manifestVersion = "0.3.0"; + pkg->getManifest().type = "ui_qml"; + pkg->getManifest().view = "qml/Main.qml"; + pkg->getManifest().icon = ""; + + fs::path testDir = tempDir / "dist"; + createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}}); + ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success); + ASSERT_TRUE(pkg->save(pkgPath).success); + + auto result = Package::verify(pkgPath); + EXPECT_TRUE(result.valid) << "0.3.0 package must remain valid. Errors: " << + (result.errors.empty() ? "none" : result.errors[0]); +} + +// Mutating the icon must change the Merkle root — assets/ is inside the tree, +// which is what makes the icon signature-covered for free. +TEST_F(PackageTest, Icon_ParticipatesInMerkleTree) { + fs::path pkgPath = tempDir / "test.lgx"; + Package::create(pkgPath, "testpkg"); + + auto pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success); + ASSERT_TRUE(pkg->save(pkgPath).success); + pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + const std::string rootA = pkg->getManifest().hashes.count("root") + ? pkg->getManifest().hashes.at("root") : ""; + ASSERT_FALSE(rootA.empty()); + + ASSERT_TRUE(pkg->setIcon(lgx_test::makePng(256, 256)).success); + // Same dimensions but different pixel payload would be identical here, so + // use a differently-sized image to guarantee different bytes. + ASSERT_TRUE(pkg->setIcon(lgx_test::makePng(128, 128)).success); + ASSERT_TRUE(pkg->save(pkgPath).success); + pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + const std::string rootB = pkg->getManifest().hashes.count("root") + ? pkg->getManifest().hashes.at("root") : ""; + + EXPECT_NE(rootA, rootB) << "assets/ must be covered by the Merkle tree"; +} + +// The gap that let the icon regression ship: every existing test checked the +// archive's CONTENTS, none checked what lands on disk after extraction. The +// manifest documents `icon` as relative to the installed package root, so the +// contract is "extract, then resolve manifest.icon" — assert exactly that. +TEST_F(PackageTest, Icon_ExtractedToInstalledPackageRoot) { + fs::path pkgPath = tempDir / "test.lgx"; + Package::create(pkgPath, "testpkg"); + + auto pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + pkg->getManifest().type = "ui_qml"; + pkg->getManifest().view = "qml/Main.qml"; + ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success); + + fs::path testDir = tempDir / "dist"; + createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}}); + ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success); + ASSERT_TRUE(pkg->save(pkgPath).success); + + fs::path out = tempDir / "installed"; + auto loaded = Package::load(pkgPath); + ASSERT_TRUE(loaded.has_value()); + ASSERT_TRUE(loaded->extractVariant("linux-amd64", out).success); + + // This is the exact join UIPluginManager::pluginIconUrl() performs. + const fs::path installDir = out / "linux-amd64"; + const fs::path resolved = installDir / loaded->getManifest().icon; + EXPECT_TRUE(fs::exists(resolved)) + << "manifest icon '" << loaded->getManifest().icon + << "' must resolve under the installed package root"; + + // Variant payload still lands where it always did. + EXPECT_TRUE(fs::exists(installDir / "qml" / "Main.qml")); + + // And the extracted icon is byte-identical to what was packaged. + std::ifstream f(resolved, std::ios::binary); + std::vector onDisk((std::istreambuf_iterator(f)), + std::istreambuf_iterator()); + EXPECT_EQ(onDisk, lgx_test::makePng()); +} + +// Assets are variant-independent: extracting a different variant must still +// produce the icon, or a darwin install would lose what a linux install kept. +TEST_F(PackageTest, Icon_ExtractedForEveryVariant) { + fs::path pkgPath = tempDir / "test.lgx"; + Package::create(pkgPath, "testpkg"); + + auto pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success); + + fs::path a = tempDir / "a"; createTestFile(a / "mod.so", "x"); + fs::path b = tempDir / "b"; createTestFile(b / "mod.dylib", "y"); + ASSERT_TRUE(pkg->addVariant("linux-amd64", a, "mod.so").success); + ASSERT_TRUE(pkg->addVariant("darwin-arm64", b, "mod.dylib").success); + ASSERT_TRUE(pkg->save(pkgPath).success); + + auto loaded = Package::load(pkgPath); + ASSERT_TRUE(loaded.has_value()); + for (const std::string v : {"linux-amd64", "darwin-arm64"}) { + fs::path out = tempDir / ("out-" + v); + ASSERT_TRUE(loaded->extractVariant(v, out).success) << v; + EXPECT_TRUE(fs::exists(out / v / "assets" / "icon.png")) + << "variant " << v << " lost the root asset"; + } +} + +// Review fix: a 0.4.0 package could previously pass with `icon` pointing +// anywhere, contradicting the canonical-path contract that hosts and the +// release tool both rely on. +TEST_F(PackageTest, Icon_NonCanonicalPathRejected) { + fs::path pkgPath = tempDir / "test.lgx"; + Package::create(pkgPath, "testpkg"); + + auto pkg = Package::load(pkgPath); + ASSERT_TRUE(pkg.has_value()); + pkg->getManifest().type = "ui_qml"; + pkg->getManifest().view = "qml/Main.qml"; + ASSERT_TRUE(pkg->setIcon(lgx_test::makePng()).success); + // Point the manifest somewhere else under assets/ — file still exists. + pkg->getManifest().icon = "assets/elsewhere.png"; + + fs::path testDir = tempDir / "dist"; + createTestDirectory(testDir, {{"qml/Main.qml", "import QtQuick\nItem {}"}}); + ASSERT_TRUE(pkg->addVariant("linux-amd64", testDir).success); + pkg->save(pkgPath); + + auto result = Package::verify(pkgPath); + EXPECT_FALSE(result.valid); + ASSERT_FALSE(result.errors.empty()); + EXPECT_NE(result.errors[0].find("assets/icon.png"), std::string::npos); +} diff --git a/tests/test_png.h b/tests/test_png.h new file mode 100644 index 0000000..9956e62 --- /dev/null +++ b/tests/test_png.h @@ -0,0 +1,84 @@ +#pragma once + +// Shared PNG fixture builder for tests that need a package icon. +// +// Package::validateIconAsset() only reads IHDR (width/height at fixed +// offsets) and never decodes pixels, but we emit a real zlib-compressed +// IDAT so fixtures are decodable images rather than header-shaped stubs — +// otherwise a test could pass against a validator that later starts +// decoding, and we would not find out here. + +#include +#include +#include +#include + +#include + +namespace lgx_test { + +// Default size is the 256x256 the Logos icon standard requires; pass another +// size to exercise rejection. +inline std::vector makePng(uint32_t w = 256, uint32_t h = 256) { + auto be32 = [](uint32_t v) { + return std::vector{ + static_cast((v >> 24) & 0xFF), + static_cast((v >> 16) & 0xFF), + static_cast((v >> 8) & 0xFF), + static_cast(v & 0xFF)}; + }; + auto chunk = [&](const std::string& tag, + const std::vector& payload) { + std::vector out = be32(static_cast(payload.size())); + std::vector tagged(tag.begin(), tag.end()); + tagged.insert(tagged.end(), payload.begin(), payload.end()); + out.insert(out.end(), tagged.begin(), tagged.end()); + const uint32_t crc = static_cast( + ::crc32(0L, tagged.data(), static_cast(tagged.size()))); + const auto crcBytes = be32(crc); + out.insert(out.end(), crcBytes.begin(), crcBytes.end()); + return out; + }; + + std::vector png = {0x89, 'P', 'N', 'G', '\r', '\n', 0x1A, '\n'}; + + std::vector ihdr = be32(w); + const auto hh = be32(h); + ihdr.insert(ihdr.end(), hh.begin(), hh.end()); + ihdr.insert(ihdr.end(), {8, 6, 0, 0, 0}); // 8-bit RGBA + const auto ihdrChunk = chunk("IHDR", ihdr); + png.insert(png.end(), ihdrChunk.begin(), ihdrChunk.end()); + + // One filter byte + 4 bytes/px per row, all zero (transparent black). + std::vector raw(static_cast(h) * (1 + 4 * w), 0); + uLongf destLen = compressBound(static_cast(raw.size())); + std::vector deflated(destLen); + // Return an empty fixture on compression failure rather than emitting a + // PNG with a truncated IDAT — callers fail fast (setIcon() rejects empty + // data), so the real cause surfaces instead of a confusing decode error. + if (compress2(deflated.data(), &destLen, raw.data(), + static_cast(raw.size()), 9) != Z_OK) { + return {}; + } + deflated.resize(destLen); + const auto idat = chunk("IDAT", deflated); + png.insert(png.end(), idat.begin(), idat.end()); + + const auto iend = chunk("IEND", {}); + png.insert(png.end(), iend.begin(), iend.end()); + return png; +} + +// Write a PNG fixture to disk and return the path, for CLI tests that pass +// --icon rather than calling Package::setIcon() directly. +inline std::string writePng(const std::string& path, + uint32_t w = 256, + uint32_t h = 256) { + const auto data = makePng(w, h); + std::ofstream out(path, std::ios::binary); + out.write(reinterpret_cast(data.data()), + static_cast(data.size())); + return path; +} + +} // namespace lgx_test