From dd195d1b5e934e0d25e574f0971a5cd6fc015ec7 Mon Sep 17 00:00:00 2001 From: Dario Gabriel Lipicar Date: Thu, 27 Aug 2026 01:17:29 -0300 Subject: [PATCH] fix(deps): build against the library this module needs, and pin the wire shape exactly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI here runs `nix build -L` — the real module, linking the real logos-package-manager. The lock pinned 7c5aad9a (2026-08-22), whose DependencyTreeNode has neither `requiredSigner` nor `signerDid`, so that build does not compile: error: no member named requiredSigner in const struct DependencyTreeNode error: no member named signerDid in const struct DependencyTreeNode That is a red build, not a latent risk. And the one check would never have said so: `tests.mockCLibs = ["logos_pm"]` builds the unit tests against tests/stubs/package_manager_lib.h, a hand-maintained MIRROR of the real header. It compiles and passes against a library it never links, so the check is green while `nix build` is red and nothing connects the two. flake.nix now pins the branch carrying those fields, with a REVERT-ON-MERGE note saying exactly this. Measured: `nix build` and `nix build .#unit-tests` both pass with NO override. The backward-compatibility test moves from three absences to an EXACT KEY SET. Naming three keys only rules out the three somebody thought of, and the way an additive key fails is by turning out not to be conditional. `signerDid` is that shape: it is a property of the PACKAGE, not of the edge, so it does not become absent because an edge declared no constraint. A real capture of a satisfied pin carries `status:"installed"` and a `signerDid` together — so a test that only looked for requiredVersion/requiredSigner would not have noticed it appear on an unconstrained node. Counting and naming every key does. Proven by mutation: one unconditional extra key turns the test red. Stub and test prose follow the mechanism. What an embedded package is missing is a manifest.sig, and it can never have one because nothing but installPluginFile copies one into an install tree — not a record nobody wrote. Co-Authored-By: Claude Opus 5 --- flake.lock | 14 ++++++++------ flake.nix | 18 +++++++++++++++++- tests/stubs/package_manager_lib.h | 12 ++++++------ tests/test_package_manager.cpp | 30 +++++++++++++++++++++++++++--- 4 files changed, 58 insertions(+), 16 deletions(-) diff --git a/flake.lock b/flake.lock index 6085e9b..a293203 100644 --- a/flake.lock +++ b/flake.lock @@ -3562,15 +3562,16 @@ ] }, "locked": { - "lastModified": 1787428523, - "narHash": "sha256-WV30+aqn7MOnz9neSY1ni8AggRPROHsc3rrocx32/1U=", + "lastModified": 1787801609, + "narHash": "sha256-+tjx4kNR8/TaBx8lL+EYEsD54w1m6rvLFiGqO6oLq9s=", "owner": "logos-co", "repo": "logos-package-manager", - "rev": "7c5aad9ae59961a0e28b13ca37b2d18baf4f0e9d", + "rev": "001f04449358cfb48e0d2d91d155b75716239097", "type": "github" }, "original": { "owner": "logos-co", + "ref": "fix/flatten-must-not-mask-a-deeper-mismatch", "repo": "logos-package-manager", "type": "github" } @@ -3750,15 +3751,16 @@ ] }, "locked": { - "lastModified": 1786463662, - "narHash": "sha256-h2ex453W73qjkN8n6Bo9ywU5X22bssmyCMpQjR5LRzY=", + "lastModified": 1787798657, + "narHash": "sha256-ZwL5qeo0HuFOJ+h2ZM33QF07j8dpb47bw2laX8DvTYU=", "owner": "logos-co", "repo": "logos-package", - "rev": "3cb520c8eadde3d6d7810dee74bdd243f73f34e9", + "rev": "67b35c094b8e985fbcdc5adcd3ae175625c67c30", "type": "github" }, "original": { "owner": "logos-co", + "ref": "feat/carry-the-manifest-signature", "repo": "logos-package", "type": "github" } diff --git a/flake.nix b/flake.nix index 5fc7e4a..1c2a230 100644 --- a/flake.nix +++ b/flake.nix @@ -3,7 +3,23 @@ inputs = { logos-module-builder.url = "github:logos-co/logos-module-builder"; - logos-package-manager.url = "github:logos-co/logos-package-manager"; + # PINNED TO A BRANCH, DELIBERATELY, AND TEMPORARILY. + # + # `master` here is a logos-package-manager whose DependencyTreeNode has + # neither `requiredSigner` nor `signerDid`, so src/package_manager_impl.cpp + # does not COMPILE against it. CI runs `nix build -L` — the real module, + # linking the real library — so leaving this on master is not a latent + # problem, it is a red build. + # + # And the check would not have told anyone: `tests.mockCLibs = ["logos_pm"]` + # builds the unit tests against tests/stubs/package_manager_lib.h, which is + # a hand-maintained MIRROR of the real header. It compiles, and passes, + # against a library it never links. The stub moving with this header is the + # only thing that keeps the two honest. + # + # REVERT TO "github:logos-co/logos-package-manager" WHEN + # fix/flatten-must-not-mask-a-deeper-mismatch MERGES. + logos-package-manager.url = "github:logos-co/logos-package-manager/fix/flatten-must-not-mask-a-deeper-mismatch"; }; outputs = inputs@{ logos-module-builder, ... }: diff --git a/tests/stubs/package_manager_lib.h b/tests/stubs/package_manager_lib.h index eba3965..71c6820 100644 --- a/tests/stubs/package_manager_lib.h +++ b/tests/stubs/package_manager_lib.h @@ -27,12 +27,12 @@ enum class DependencyStatus { // rejects. Appended, never inserted — the real enum's numeric values are // ABI across libpackage_manager_lib. VersionMismatch, - // Installed, and provably NOT the package the dependant named: the key - // this install's signature verified against is not the `signer` DID on - // the edge. Identity, not trust — see the real header. + // Installed, and provably NOT the package the dependant named: the + // installed manifest.sig does not verify under the key the edge's + // `signer` DID carries. Identity, not trust — see the real header. SignerMismatch, - // The edge pins a `signer` and nothing records who published what is - // installed. Absence of evidence, reported as its own status. + // The edge pins a `signer` and no usable signature is installed to check + // it against. Absence of evidence, reported as its own status. SignerUnknown, }; @@ -61,7 +61,7 @@ struct Hashes { // Mirrors PackageDependency in package_manager_lib.h — one `dependencies[]` // entry, either a plain name or an object carrying a semver range and/or a -// publisher DID. +// signer DID. struct PackageDependency { std::string name; std::optional version; diff --git a/tests/test_package_manager.cpp b/tests/test_package_manager.cpp index 5a024bb..3d09873 100644 --- a/tests/test_package_manager.cpp +++ b/tests/test_package_manager.cpp @@ -1937,7 +1937,21 @@ LOGOS_TEST(resolveFlatDependencies_surfaces_version_mismatch) { } LOGOS_TEST(resolveDependencies_omits_constraint_keys_when_unconstrained) { - // The backward-compatibility half for the tree wire format. + // The backward-compatibility half for the tree wire format, pinned as an + // EXACT KEY SET rather than as a few absences. + // + // Three `contains` checks only rule out the three keys somebody thought to + // name. Every key added here from now on is additive by intention, and the + // way that intention fails is a key that turns out NOT to be conditional — + // emitted unconditionally, or conditional on something that is true for an + // ordinary package. `signerDid` is exactly that shape: it is a property of + // the PACKAGE, not of the edge, so it does not become absent just because + // this edge declared no constraint, and a test that only looked for + // requiredVersion/requiredSigner would not have noticed it appear here. + // + // So: count and name every key. A node with no constraints and no + // signature carries these five and nothing else, which is byte-for-byte + // what this API emitted before any of this work. auto t = LogosTestContext("package_manager"); setMockDependencyTree(makeForwardTree()); @@ -1946,8 +1960,17 @@ LOGOS_TEST(resolveDependencies_omits_constraint_keys_when_unconstrained) { LogosMap out = impl.resolveDependencies("root", true); LogosMap dep = out["children"][0]; LOGOS_ASSERT_EQ(dep["status"].get(), std::string("installed")); + LOGOS_ASSERT_EQ(dep.size(), static_cast(5)); + LOGOS_ASSERT_TRUE(dep.contains("name")); + LOGOS_ASSERT_TRUE(dep.contains("status")); + LOGOS_ASSERT_TRUE(dep.contains("version")); + LOGOS_ASSERT_TRUE(dep.contains("installType")); + LOGOS_ASSERT_TRUE(dep.contains("children")); + // Named individually as well, so a failure says WHICH key appeared rather + // than only that the count moved. LOGOS_ASSERT_FALSE(dep.contains("requiredVersion")); LOGOS_ASSERT_FALSE(dep.contains("requiredSigner")); + LOGOS_ASSERT_FALSE(dep.contains("signerDid")); } LOGOS_TEST(resolveDependencies_absent_dependency_keeps_its_declared_range) { @@ -2021,7 +2044,7 @@ LOGOS_TEST(getInstalledPackages_omits_the_signer_did_when_unsigned) { LOGOS_ASSERT_FALSE(list[0].contains("signerDid")); } -// A dependency installed under the right name by the WRONG publisher. +// A dependency installed under the right name, signed by the WRONG KEY. static DependencyTreeNode makeSignerMismatchTree() { DependencyTreeNode root; root.name = "app"; @@ -2097,7 +2120,8 @@ LOGOS_TEST(resolveDependencies_surfaces_signer_unknown_without_a_signer_did) { LOGOS_ASSERT_EQ(dep["requiredSigner"].get(), std::string("did:jwk:PINNED")); LOGOS_ASSERT_FALSE(dep.contains("signerDid")); // Still on disk — and an embedded package is the population that can NEVER - // acquire a recorded publisher, because it never passes through install. + // carry a signature, because it never passes through installPluginFile, + // which is the only thing that copies a manifest.sig into an install tree. LOGOS_ASSERT_EQ(dep["version"].get(), std::string("1.0.0")); LOGOS_ASSERT_EQ(dep["installType"].get(), std::string("embedded")); }