Add the service archetype and a builder for a rented host #60

Closed
allod-agent wants to merge 6 commits from agent/service-archetype into master
Member

service becomes the fourth archetype, beside dev, privacy and hypervisor, and mkServiceVm builds one: a rented, internet-facing host that this fleet did not create and does not run. Nothing changes for any machine that exists today — no machine has type = "service", every existing machine's generated configuration is untouched, and the repo's own gate confirms it. What proves the new archetype works is a check that drives the production builder with a fixture and forces the machine it generates. If it is wrong, revert the commit; no machine is built from this yet.

An adversarial review pass by a second model found seven issues after the first commit; the fixes are in 8e6f872 and the findings are recorded in a comment below. Two mattered: the builder was defaulting root SSH on a rented host to the nexus host identity, and the check's central claim was false because Nix laziness hid the reads it claimed to forbid.

The builder

Shaped like mkHypervisor in the one respect that matters: nothing in this fleet runs the machine, so there is no guest module to select and no runtime to read. It differs from a hypervisor in owning no guests, so the microvmGuests machinery does not apply either.

The builder lives in nix/service-builder.nix, not inline beside its siblings: its parameters are lib, nixosSystem and publicHostModule, so inventory is not in its lexical scope and an accidental read inside it fails to evaluate. That boundary stops at the call site — parameter names are pinned by the check, values are not — so it is a guardrail against drift, not a sandbox; the PR comments record the review round that established exactly where it stops. It composes nixosModules.publicHost and nothing else, and takes the hostname from the machine's identity. Everything that makes the machine a particular service arrives through the profile definition:

  • nixosModules.staticSite is composed by the definition, not the builder. A builder that imported it would be deciding that every service machine serves a static site.
  • nixosModules.rentedKvmGuest is likewise absent, for the reason its own header gives: the boot and disk shape it carries is a measurement of one provider rather than of what an internet-facing machine needs.

platform and adminAuthorizedKeys are required, with no default; stateVersion carries one. platform is required so that mkServiceVm mentions machines nowhere at all — machineConfigurations supplies it, the way it supplies the dev-only pi contract — which makes "this builder reads no inventory" true by construction rather than by a check. adminAuthorizedKeys is required because there is no framework-wide operator key set that is correct for a machine somebody else hosts: secrets.lib.identity.hostPublicKeys is the nexus host identity and a recipient of every ciphertext in the fleet, so defaulting to it would grant root on a rented box to the holder of the decrypt-everything key. The base leaves adminSshPort and acmeEmail undefaulted for the same reason. Both are pinned by builtins.functionArgs in the check, because a required argument that nothing pins is one edit from becoming a default again.

adminAuthorizedKeys is required, with no default. There is no framework-wide operator key set that is correct for a machine somebody else hosts — secrets.lib.identity.hostPublicKeys is the nexus host identity and a recipient of every ciphertext in the fleet, so defaulting to it would grant root on a rented box to the holder of the decrypt-everything key. The base leaves adminSshPort and acmeEmail undefaulted for the same reason.

The archetype composes no home-manager, and normalizeDefinition hands every definition a homeModules list regardless. That is the one place this archetype could fall back silently, so a service definition declaring homeModules is an evaluation error naming why.

Three changes beyond the builder

vmFacts excludes service machines. includeMachine filtered on type != "hypervisor", so the first service machine would have landed in vmFacts, reached requireRuntime, and aborted with "missing runtime" — the exact confusing downstream failure allod/inventory#13 exists to prevent.

runtime-module-selection had the same stale filter, found by the review pass: its guestMachines was still every non-hypervisor, and it reads machine.runtime, which the new schema forbids on a service machine. Both that filter and its hypervisorLeaks complement now derive from vmFactsLib.includeMachine, so this repo holds one copy of the predicate instead of three.

profile-definition-contracts no longer uses service as its unknown-archetype fixture. It did, and adding the archetype turned that check red for exactly the right reason: a fixture named after a plausible future archetype stops testing the guard the day that archetype is added. The fixture is now a synthetic fixture-unknown key, and the expected archetype set is written out and asserted equal to profileArchetypes — an intermediate version derived it from profileArchetypes instead, which the review showed left the check green when that list was corrupted.

Depends on

allod/inventory#13 (PR allod/inventory#14) must land before a real service machine can be described. The review confirmed by hash that the currently locked inventory rejects a valid service machine and that the revision in that PR accepts it. This repo evaluates fine against the current lock because the builder reads no runtime and the check overrides platform, so the two PRs are independently landable — but the archetype is not usable until the lock bumps.

Two things should land with that bump: deleting this repo's copy of the runtime-free type list in favour of inventory's new lib.isGuestMachine export, and making the dependency load-bearing by referencing inventory's service-machine-mutations check the way runtime-module-selection already references runtime-fact-mutations.

Risk

R1. Additive: a new archetype name, a new builder, and a new check. The changes to existing code are the two guest-filter fixes, both no-ops while no service machine exists, and a check fixture repair.

The builder is written against zero deployed machines, which is what the note it replaces warned about. That objection is answered by shape rather than by evidence: the shape is mkHypervisor's, which has booted, and the builder is small enough to have very little API to freeze. Making adminAuthorizedKeys required rather than defaulted is the one place this PR deliberately refuses to invent a shape.

Adding the first service machine still needs an identity in secretsmachineConfigurations asserts machine names and identity names match exactly — and an inventory input new enough to accept the type. Both fail loudly.

Validation

./check.sh, this repo's full gate, passes: all 32 steps — every machine toplevel, every exported module, and all 19 checks, including the new service-archetype.

The check is exercised by sabotage rather than trusted:

  • Removing the homeModules assertion turns the sabotage finding red.
  • Adding environment.variables.SERVICE_INVENTORY_IP = machines.${name}.ip to the builder — the review's own sabotage, which defeated the previous version of this check — now fails with attribute 'service-fixture' missing.
  • Corrupting profileArchetypes now turns profile-definition-contracts red, naming both lists.

The check drives fixtures through builders.service, not through mkServiceVm directly, so every finding is also evidence that the registration points at the builder under test. An earlier draft compared builders.service == mkServiceVm; Nix does not compare functions, so that comparison was silently false and is gone.

Follow-up, not in this PR

allod/profiles/flake.nix:27 carries a comment reading "Archetype-name validity (dev|privacy|hypervisor)", which this change makes stale. It is a comment in a third repo and asserts nothing; worth a one-line fix on its own.

Closes allod/archetypes#59
Refs allod/inventory#13

`service` becomes the fourth archetype, beside `dev`, `privacy` and `hypervisor`, and `mkServiceVm` builds one: a rented, internet-facing host that this fleet did not create and does not run. Nothing changes for any machine that exists today — no machine has `type = "service"`, every existing machine's generated configuration is untouched, and the repo's own gate confirms it. What proves the new archetype works is a check that drives the production builder with a fixture and forces the machine it generates. If it is wrong, revert the commit; no machine is built from this yet. An adversarial review pass by a second model found seven issues after the first commit; the fixes are in `8e6f872` and the findings are recorded in a comment below. Two mattered: the builder was defaulting root SSH on a rented host to the nexus host identity, and the check's central claim was false because Nix laziness hid the reads it claimed to forbid. ## The builder Shaped like `mkHypervisor` in the one respect that matters: nothing in this fleet runs the machine, so there is no guest module to select and no `runtime` to read. It differs from a hypervisor in owning no guests, so the `microvmGuests` machinery does not apply either. The builder lives in `nix/service-builder.nix`, not inline beside its siblings: its parameters are `lib`, `nixosSystem` and `publicHostModule`, so inventory is not in its lexical scope and an accidental read inside it fails to evaluate. That boundary stops at the call site — parameter names are pinned by the check, values are not — so it is a guardrail against drift, not a sandbox; the PR comments record the review round that established exactly where it stops. It composes `nixosModules.publicHost` and nothing else, and takes the hostname from the machine's identity. Everything that makes the machine a particular service arrives through the profile definition: - `nixosModules.staticSite` is composed by the definition, not the builder. A builder that imported it would be deciding that every service machine serves a static site. - `nixosModules.rentedKvmGuest` is likewise absent, for the reason its own header gives: the boot and disk shape it carries is a measurement of one provider rather than of what an internet-facing machine needs. `platform` and `adminAuthorizedKeys` are **required, with no default**; `stateVersion` carries one. `platform` is required so that `mkServiceVm` mentions `machines` nowhere at all — `machineConfigurations` supplies it, the way it supplies the dev-only pi contract — which makes "this builder reads no inventory" true by construction rather than by a check. `adminAuthorizedKeys` is required because there is no framework-wide operator key set that is correct for a machine somebody else hosts: `secrets.lib.identity.hostPublicKeys` is the nexus host identity and a recipient of every ciphertext in the fleet, so defaulting to it would grant root on a rented box to the holder of the decrypt-everything key. The base leaves `adminSshPort` and `acmeEmail` undefaulted for the same reason. Both are pinned by `builtins.functionArgs` in the check, because a required argument that nothing pins is one edit from becoming a default again. `adminAuthorizedKeys` is **required, with no default**. There is no framework-wide operator key set that is correct for a machine somebody else hosts — `secrets.lib.identity.hostPublicKeys` is the nexus host identity and a recipient of every ciphertext in the fleet, so defaulting to it would grant root on a rented box to the holder of the decrypt-everything key. The base leaves `adminSshPort` and `acmeEmail` undefaulted for the same reason. The archetype composes no home-manager, and `normalizeDefinition` hands every definition a `homeModules` list regardless. That is the one place this archetype could fall back silently, so a service definition declaring `homeModules` is an evaluation error naming why. ## Three changes beyond the builder **`vmFacts` excludes service machines.** `includeMachine` filtered on `type != "hypervisor"`, so the first service machine would have landed in `vmFacts`, reached `requireRuntime`, and aborted with "missing runtime" — the exact confusing downstream failure `allod/inventory#13` exists to prevent. **`runtime-module-selection` had the same stale filter**, found by the review pass: its `guestMachines` was still every non-hypervisor, and it reads `machine.runtime`, which the new schema forbids on a service machine. Both that filter and its `hypervisorLeaks` complement now derive from `vmFactsLib.includeMachine`, so this repo holds one copy of the predicate instead of three. **`profile-definition-contracts` no longer uses `service` as its unknown-archetype fixture.** It did, and adding the archetype turned that check red for exactly the right reason: a fixture named after a plausible future archetype stops testing the guard the day that archetype is added. The fixture is now a synthetic `fixture-unknown` key, and the expected archetype set is written out and asserted equal to `profileArchetypes` — an intermediate version derived it from `profileArchetypes` instead, which the review showed left the check green when that list was corrupted. ## Depends on `allod/inventory#13` (PR allod/inventory#14) must land before a real service machine can be described. The review confirmed by hash that the currently locked inventory rejects a valid service machine and that the revision in that PR accepts it. This repo evaluates fine against the current lock because the builder reads no `runtime` and the check overrides `platform`, so the two PRs are independently landable — but the archetype is not usable until the lock bumps. Two things should land with that bump: deleting this repo's copy of the runtime-free type list in favour of inventory's new `lib.isGuestMachine` export, and making the dependency load-bearing by referencing inventory's `service-machine-mutations` check the way `runtime-module-selection` already references `runtime-fact-mutations`. ## Risk R1. Additive: a new archetype name, a new builder, and a new check. The changes to existing code are the two guest-filter fixes, both no-ops while no service machine exists, and a check fixture repair. The builder is written against zero deployed machines, which is what the note it replaces warned about. That objection is answered by shape rather than by evidence: the shape is `mkHypervisor`'s, which has booted, and the builder is small enough to have very little API to freeze. Making `adminAuthorizedKeys` required rather than defaulted is the one place this PR deliberately refuses to invent a shape. Adding the first service machine still needs an identity in `secrets` — `machineConfigurations` asserts machine names and identity names match exactly — and an inventory input new enough to accept the type. Both fail loudly. ## Validation `./check.sh`, this repo's full gate, passes: all 32 steps — every machine toplevel, every exported module, and all 19 checks, including the new `service-archetype`. The check is exercised by sabotage rather than trusted: - Removing the `homeModules` assertion turns the sabotage finding red. - Adding `environment.variables.SERVICE_INVENTORY_IP = machines.${name}.ip` to the builder — the review's own sabotage, which defeated the previous version of this check — now fails with `attribute 'service-fixture' missing`. - Corrupting `profileArchetypes` now turns `profile-definition-contracts` red, naming both lists. The check drives fixtures through `builders.service`, not through `mkServiceVm` directly, so every finding is also evidence that the registration points at the builder under test. An earlier draft compared `builders.service == mkServiceVm`; Nix does not compare functions, so that comparison was silently false and is gone. ## Follow-up, not in this PR `allod/profiles/flake.nix:27` carries a comment reading "Archetype-name validity (dev|privacy|hypervisor)", which this change makes stale. It is a comment in a third repo and asserts nothing; worth a one-line fix on its own. Closes allod/archetypes#59 Refs allod/inventory#13
Author
Member

Adversarial review pass (gpt-5.6-sol, xhigh) — findings and fixes

An independent read-only review was run against 4007fae with a different model, driven to sabotage production code and observe whether the checks went red. It found seven issues. Five are fixed in 8e6f872, one is answered, one is accepted as a documented limit.

1. High, fixed — the builder defaulted root SSH on a rented host to the nexus host identity. adminAuthorizedKeys defaulted to secrets.lib.identity.hostPublicKeys, and the PR called those "the owner's keys". They are not: secrets binds that value to hypervisorPublicKeys, and it is a recipient of every ciphertext in the fleet. The default would have handed root on a rented, internet-facing, third-party-hosted machine to the holder of the key that decrypts everything — silently, because a default is what you get by saying nothing. That widens what one key authorizes in the direction principle 7 exists to prevent.

adminAuthorizedKeys is now a required argument with no default. There is no framework-wide operator key set that is correct for a machine somebody else hosts, so the framework does not guess — the same position the base already takes on adminSshPort and acmeEmail.

2. High, fixed — the check's central claim was false. It argued that because the fixture name is absent from machines, any stray machines.${name}.<field> read would be an attribute error. It would — but only if something forced it, and reading a handful of options does not. The reviewer added environment.variables.SERVICE_INVENTORY_IP = machines.${name}.ip to the builder and the check stayed green.

The check now composes a complete machine (with rentedKvmGuest, the way a real service definition would) and forces its toplevel derivation at evaluation time. That exact sabotage now fails with attribute 'service-fixture' missing.

One correction worth recording, because the first attempt at this fix was wrong in an expensive way: interpolating the drvPath into the runCommand script makes the check depend on the built machine, turning a seconds-long evaluation into a full NixOS system build. It is forced with builtins.seq in an assert instead, which instantiates without realising anything. The check header says so.

3. High, fixed — runtime-module-selection would have aborted on the first service machine. Its guestMachines filter was still type != "hypervisor", so a rented host landed in it and declaredMismatches read machine.runtime — a field the new schema forbids. This is the same bug class fixed in vm-facts.nix in the first commit and missed here. Both that filter and the hypervisorLeaks complement now derive from vmFactsLib.includeMachine, so there is one predicate in this repo rather than three.

allod/inventory#14 now also exports lib.runtimeFreeTypes / lib.isGuestMachine, since the classification is a machine fact and inventory owns machine facts. This repo's copy is marked for deletion at the lock bump.

4. Medium, fixed — the archetype-contract witness derived its expectation from the value under test. Replacing privacy with a bogus name in profileArchetypes left profile-definition-contracts green: the lib.genAttrs profileArchetypes fixture followed the corruption. My earlier "fix" for the service collision traded one fragility for another. The expected set is now written out and profileArchetypes is asserted equal to it (as a sorted set), so the production list is answerable to the check rather than defining it. Reproduced red after the fix.

5. Low, fixed — stale documentation. The nixosModules comment still said a rented host is not in inventory and can only consume a module. Rewritten to describe both paths, since exporting the modules still matters for machines outside this inventory entirely.

6. Medium, partly accepted — check size. The reviewer called the check gold-plating against an issue that says "expect this to be small", specifically for re-asserting the base's Caddy/ACME/sshd/fail2ban/firewall posture that checks/public-host.nix already owns. Agreed and trimmed: those assertions are gone, leaving two probes that the base is composed at all. The forced toplevel is new cost in the other direction, and it stays — principle 13 puts a generated artifact above a source argument, and finding 2 is exactly what inspecting options instead of forcing them buys.

7. Not changed — the identity join. The reviewer notes that adding a real service machine fails with machines keys must exactly match identity keys, because no service identity source exists. That is a loud failure at a documented boundary, and both issues scope out "any actual service machine": the first one is added with its identity, in secrets, when its host is rented. The README now states the two things the first service machine still needs, rather than leaving them to be discovered.

Remaining dependency

The reviewer confirmed by hash that the locked inventory revision rejects a valid service machine (inventory machines missing runtime: service-1), and that the revision in allod/inventory#14 accepts it. That is the lock bump already noted in the PR body. Their suggestion to make the dependency load-bearing — referencing inventory's service-machine-mutations check the way runtime-module-selection references runtime-fact-mutations — should land with that bump, since the check does not exist in the currently locked revision.

Validation

./check.sh passes: all 32 steps.

## Adversarial review pass (gpt-5.6-sol, xhigh) — findings and fixes An independent read-only review was run against `4007fae` with a different model, driven to sabotage production code and observe whether the checks went red. It found seven issues. Five are fixed in `8e6f872`, one is answered, one is accepted as a documented limit. **1. High, fixed — the builder defaulted root SSH on a rented host to the nexus host identity.** `adminAuthorizedKeys` defaulted to `secrets.lib.identity.hostPublicKeys`, and the PR called those "the owner's keys". They are not: `secrets` binds that value to `hypervisorPublicKeys`, and it is a recipient of every ciphertext in the fleet. The default would have handed root on a rented, internet-facing, third-party-hosted machine to the holder of the key that decrypts everything — silently, because a default is what you get by saying nothing. That widens what one key authorizes in the direction principle 7 exists to prevent. `adminAuthorizedKeys` is now a required argument with no default. There is no framework-wide operator key set that is correct for a machine somebody else hosts, so the framework does not guess — the same position the base already takes on `adminSshPort` and `acmeEmail`. **2. High, fixed — the check's central claim was false.** It argued that because the fixture name is absent from `machines`, any stray `machines.${name}.<field>` read would be an attribute error. It would — but only if something forced it, and reading a handful of options does not. The reviewer added `environment.variables.SERVICE_INVENTORY_IP = machines.${name}.ip` to the builder and the check stayed green. The check now composes a complete machine (with `rentedKvmGuest`, the way a real service definition would) and forces its toplevel derivation at evaluation time. That exact sabotage now fails with `attribute 'service-fixture' missing`. One correction worth recording, because the first attempt at this fix was wrong in an expensive way: interpolating the drvPath into the `runCommand` script makes the check *depend* on the built machine, turning a seconds-long evaluation into a full NixOS system build. It is forced with `builtins.seq` in an assert instead, which instantiates without realising anything. The check header says so. **3. High, fixed — `runtime-module-selection` would have aborted on the first service machine.** Its `guestMachines` filter was still `type != "hypervisor"`, so a rented host landed in it and `declaredMismatches` read `machine.runtime` — a field the new schema forbids. This is the same bug class fixed in `vm-facts.nix` in the first commit and missed here. Both that filter and the `hypervisorLeaks` complement now derive from `vmFactsLib.includeMachine`, so there is one predicate in this repo rather than three. `allod/inventory#14` now also exports `lib.runtimeFreeTypes` / `lib.isGuestMachine`, since the classification is a machine fact and inventory owns machine facts. This repo's copy is marked for deletion at the lock bump. **4. Medium, fixed — the archetype-contract witness derived its expectation from the value under test.** Replacing `privacy` with a bogus name in `profileArchetypes` left `profile-definition-contracts` green: the `lib.genAttrs profileArchetypes` fixture followed the corruption. My earlier "fix" for the `service` collision traded one fragility for another. The expected set is now written out and `profileArchetypes` is asserted equal to it (as a sorted set), so the production list is answerable to the check rather than defining it. Reproduced red after the fix. **5. Low, fixed — stale documentation.** The `nixosModules` comment still said a rented host is not in inventory and can only consume a module. Rewritten to describe both paths, since exporting the modules still matters for machines outside this inventory entirely. **6. Medium, partly accepted — check size.** The reviewer called the check gold-plating against an issue that says "expect this to be small", specifically for re-asserting the base's Caddy/ACME/sshd/fail2ban/firewall posture that `checks/public-host.nix` already owns. Agreed and trimmed: those assertions are gone, leaving two probes that the base is composed at all. The forced toplevel is new cost in the other direction, and it stays — principle 13 puts a generated artifact above a source argument, and finding 2 is exactly what inspecting options instead of forcing them buys. **7. Not changed — the identity join.** The reviewer notes that adding a real service machine fails with `machines keys must exactly match identity keys`, because no service identity source exists. That is a loud failure at a documented boundary, and both issues scope out "any actual service machine": the first one is added with its identity, in `secrets`, when its host is rented. The README now states the two things the first service machine still needs, rather than leaving them to be discovered. ## Remaining dependency The reviewer confirmed by hash that the locked `inventory` revision rejects a valid service machine (`inventory machines missing runtime: service-1`), and that the revision in `allod/inventory#14` accepts it. That is the lock bump already noted in the PR body. Their suggestion to make the dependency load-bearing — referencing inventory's `service-machine-mutations` check the way `runtime-module-selection` references `runtime-fact-mutations` — should land with that bump, since the check does not exist in the currently locked revision. ## Validation `./check.sh` passes: all 32 steps.
Author
Member

Rounds two and three — the fixes reviewed by two models

8e6f872 was reviewed twice more: once by gpt-5.6-sol and once by gpt-5.5, both targeting the fix commit rather than the original change, because those fixes were written by the same author who wrote the defects. Four defects came out of it, all reproduced. Three are fixed in 6666b77; one is open and described at the end.

1. High, fixed — nothing pinned that adminAuthorizedKeys stays required. Every fixture in the check passes it explicitly, so restoring the exact dangerous default — ? secrets.lib.identity.hostPublicKeys, the one that would authorize the fleet's decrypt-everything identity for root on a rented host — left the check green. The security fix was one careless edit from silently reverting.

The builder's signature is now pinned directly with builtins.functionArgs: adminAuthorizedKeys and platform must both carry no default. Restoring the old default now fails with a message naming what the default would do.

2. Medium, fixed — the guest-filter fix had no witness. Reverting guestMachines to type != "hypervisor" left runtime-module-selection green, because no service machine exists to trip it. Same shape as finding 1: the fix was right, and nothing held it there. There is now a guestMachinesFor helper driven by a synthetic fixture of one dev, one service and one hypervisor machine; reverting the predicate fails with the guest discriminator selected ["fixture-dev","fixture-service"]. Both models raised this independently.

3. Low, fixed — the check was coupled to rentedKvmGuest. Its complete-machine fixture imported the disk module, so making one of that module's options required would have failed this check for a reason with nothing to do with the service archetype. It now uses a minimal inline disk fixture. checks/public-host.nix remains where the rented-guest disk shape is pinned, which both reviewers confirmed.

4. Fixed — an assertion with an unreachable false branch. lib.assertMsg (builtins.seq x true) either returns true or propagates, so its custom message could never print. Replaced with a plain builtins.seq.

The claim that kept being wrong, and what replaced it

Round one showed the check's "the builder reads nothing else from inventory" claim was false because unforced reads are invisible. The fix was to force the toplevel. Round two showed the forced toplevel misses a lazy sibling such as system.build.someAttr. A claim that needs a cleverer check each time it is falsified is the wrong shape, so it is no longer defended by a check at all.

mkServiceVm now takes platform as a required argument and mentions machines nowhere in code; machineConfigurations supplies the value, the way it supplies the dev-only pi contract. The property holds because the builder has no reason to read inventory, not because a check can catch it — principle 4's deepest available layer. The check pins the signature that makes it so.

Open, and the reason it is open

gpt-5.5 found a residual case even after that: a future edit could add an inventory read to the builder's specialArgs, and neither the forced toplevel nor the signature pin would catch it. This is not a live defect — the builder reads no inventory today, verified — but nothing prevents that edit either.

The right fix is structural rather than another check: move mkServiceVm into its own file taking explicit parameters, so machines is genuinely out of lexical scope and reading it requires adding a visible parameter. That is the pattern allod/memory testing.md already records for modules/microvm-credential-hook.nix. It is a refactor of code written in this PR and is deliberately not being done half-finished at the end of a session; it should land as its own commit here before merge.

Convergence

The two models agree on the disposition of every earlier finding: F1, F3 and F4 closed, F2 partially closed for exactly the reason above. gpt-5.5 found no defect gpt-5.6-sol had missed except the specialArgs case, and confirmed independently that the trimmed base assertions are still covered by checks/public-host.nix, that the forced toplevel realises nothing (about 5-6s warm), and that all existing machine configurations still instantiate.

Validation

./check.sh passes: all 32 steps.

## Rounds two and three — the fixes reviewed by two models `8e6f872` was reviewed twice more: once by `gpt-5.6-sol` and once by `gpt-5.5`, both targeting the fix commit rather than the original change, because those fixes were written by the same author who wrote the defects. Four defects came out of it, all reproduced. Three are fixed in `6666b77`; one is open and described at the end. **1. High, fixed — nothing pinned that `adminAuthorizedKeys` stays required.** Every fixture in the check passes it explicitly, so restoring the exact dangerous default — `? secrets.lib.identity.hostPublicKeys`, the one that would authorize the fleet's decrypt-everything identity for root on a rented host — left the check green. The security fix was one careless edit from silently reverting. The builder's signature is now pinned directly with `builtins.functionArgs`: `adminAuthorizedKeys` and `platform` must both carry no default. Restoring the old default now fails with a message naming what the default would do. **2. Medium, fixed — the guest-filter fix had no witness.** Reverting `guestMachines` to `type != "hypervisor"` left `runtime-module-selection` green, because no service machine exists to trip it. Same shape as finding 1: the fix was right, and nothing held it there. There is now a `guestMachinesFor` helper driven by a synthetic fixture of one dev, one service and one hypervisor machine; reverting the predicate fails with `the guest discriminator selected ["fixture-dev","fixture-service"]`. Both models raised this independently. **3. Low, fixed — the check was coupled to `rentedKvmGuest`.** Its complete-machine fixture imported the disk module, so making one of that module's options required would have failed this check for a reason with nothing to do with the service archetype. It now uses a minimal inline disk fixture. `checks/public-host.nix` remains where the rented-guest disk shape is pinned, which both reviewers confirmed. **4. Fixed — an assertion with an unreachable false branch.** `lib.assertMsg (builtins.seq x true)` either returns true or propagates, so its custom message could never print. Replaced with a plain `builtins.seq`. ## The claim that kept being wrong, and what replaced it Round one showed the check's "the builder reads nothing else from inventory" claim was false because unforced reads are invisible. The fix was to force the toplevel. Round two showed the forced toplevel misses a lazy sibling such as `system.build.someAttr`. A claim that needs a cleverer check each time it is falsified is the wrong shape, so it is no longer defended by a check at all. `mkServiceVm` now takes `platform` as a required argument and mentions `machines` nowhere in code; `machineConfigurations` supplies the value, the way it supplies the dev-only pi contract. The property holds because the builder has no reason to read inventory, not because a check can catch it — principle 4's deepest available layer. The check pins the signature that makes it so. ## Open, and the reason it is open `gpt-5.5` found a residual case even after that: a future edit could add an inventory read to the builder's `specialArgs`, and neither the forced toplevel nor the signature pin would catch it. This is not a live defect — the builder reads no inventory today, verified — but nothing prevents that edit either. The right fix is structural rather than another check: move `mkServiceVm` into its own file taking explicit parameters, so `machines` is genuinely out of lexical scope and reading it requires adding a visible parameter. That is the pattern `allod/memory` `testing.md` already records for `modules/microvm-credential-hook.nix`. It is a refactor of code written in this PR and is deliberately not being done half-finished at the end of a session; it should land as its own commit here before merge. ## Convergence The two models agree on the disposition of every earlier finding: F1, F3 and F4 closed, F2 partially closed for exactly the reason above. `gpt-5.5` found no defect `gpt-5.6-sol` had missed except the `specialArgs` case, and confirmed independently that the trimmed base assertions are still covered by `checks/public-host.nix`, that the forced toplevel realises nothing (about 5-6s warm), and that all existing machine configurations still instantiate. ## Validation `./check.sh` passes: all 32 steps.
Author
Member

Round four — the refactor reviewed, and its thesis broken

82bea7a moved mkServiceVm into nix/service-builder.nix so that inventory is not in its lexical scope, and claimed the "this builder reads no inventory" property was thereby held by construction. A fourth review round was run against that commit alone, asked specifically to break the thesis. It did, and the claim is corrected in 315ba08.

The attack, reproduced independently before acting on it. A two-file edit — lib = lib // { inherit machines; } at the call site in flake.nix, and lib.machines.allod-dev.platform inside the builder — reads inventory while builtins.functionArgs still returns exactly [ "lib" "nixosSystem" "publicHostModule" ]. Every assertion in service-archetype stayed green and the check evaluated to its baseline derivation.

So the guarantee was overstated. Parameter names are pinned; parameter values are not, and cannot usefully be: every parameter is the same kind of channel, and nixosSystem could equally be a closure that captured the machine set. lib is not a value a check can meaningfully constrain.

No new machinery was added, deliberately. Pinning publicHostModule by path — the one comparable value of the three — would leave two channels open while looking like a fix. That is the "cleverer check each time the claim is falsified" pattern this PR already identified as the wrong shape, applied one layer further out. The claim was corrected instead, in all four places it was overstated: the builder file's header, the check's header, the service-archetype capability-pin comment, and the README.

The boundary, stated accurately:

  • An accidental single-file read is impossible. Round three's specialArgs sabotage fails with undefined variable 'machines' rather than needing to be caught.
  • Adding an inventory parameter is caught: the check pins the parameter-name list, and re-adding machines fails with the builder file's capability list is ["lib","machines","nixosSystem","publicHostModule"], expected [...].
  • Routing inventory through an existing parameter's value is caught by nothing. It requires a two-file edit that is glaring in review, and that is the whole of its defence.

That is a guardrail against drift, not a sandbox, and it matches the threat model architecture.md states — agent error and drift, with hooks as guardrails against the former (principle 14). It is written down so the next reader does not rediscover it, which three readers of three previous claims had to.

Verified directly rather than taken on the reviewer's word

The review terminated early — it moved from reviewing code to probing the Nix fetcher cache and sandbox boundaries, which tripped a provider-side cybersecurity filter — so its report covers the two attacks and nothing else. The remaining questions were checked here:

  • No behaviour drift. allod-dev, nexus, privacy-1 and installer produce byte-identical toplevel derivation paths across HEAD~1..HEAD. The refactor moved code and changed no generated output.
  • The other pins still fire. Restoring a default on adminAuthorizedKeys and removing the homeModules assertion each turn service-archetype red with their own diagnostics.
  • ./check.sh passes: all 32 steps.

Still open, and a judgement call rather than a defect

The review was cut off before answering whether the refactor is proportionate. mkServiceVm is now the only builder in its own file; mkDevVm, mkPrivacyVm and mkHypervisor remain inline in flake.nix, and that inconsistency has a real cost in surprise. The case for it is that this is the only builder whose correctness argument depends on what it cannot reach, and allod/memory testing.md already records the same shape for modules/microvm-credential-hook.nix. The case against is that it buys a boundary which, as above, stops at the call site. Worth a reviewer's opinion before merge; either answer is defensible and it is cheap to inline again.

## Round four — the refactor reviewed, and its thesis broken `82bea7a` moved `mkServiceVm` into `nix/service-builder.nix` so that inventory is not in its lexical scope, and claimed the "this builder reads no inventory" property was thereby held by construction. A fourth review round was run against that commit alone, asked specifically to break the thesis. It did, and the claim is corrected in `315ba08`. **The attack, reproduced independently before acting on it.** A two-file edit — `lib = lib // { inherit machines; }` at the call site in `flake.nix`, and `lib.machines.allod-dev.platform` inside the builder — reads inventory while `builtins.functionArgs` still returns exactly `[ "lib" "nixosSystem" "publicHostModule" ]`. Every assertion in `service-archetype` stayed green and the check evaluated to its baseline derivation. So the guarantee was overstated. Parameter *names* are pinned; parameter *values* are not, and cannot usefully be: every parameter is the same kind of channel, and `nixosSystem` could equally be a closure that captured the machine set. `lib` is not a value a check can meaningfully constrain. **No new machinery was added, deliberately.** Pinning `publicHostModule` by path — the one comparable value of the three — would leave two channels open while looking like a fix. That is the "cleverer check each time the claim is falsified" pattern this PR already identified as the wrong shape, applied one layer further out. The claim was corrected instead, in all four places it was overstated: the builder file's header, the check's header, the `service-archetype` capability-pin comment, and the README. **The boundary, stated accurately:** - An accidental single-file read is impossible. Round three's `specialArgs` sabotage fails with `undefined variable 'machines'` rather than needing to be caught. - Adding an inventory *parameter* is caught: the check pins the parameter-name list, and re-adding `machines` fails with `the builder file's capability list is ["lib","machines","nixosSystem","publicHostModule"], expected [...]`. - Routing inventory through an existing parameter's *value* is caught by nothing. It requires a two-file edit that is glaring in review, and that is the whole of its defence. That is a guardrail against drift, not a sandbox, and it matches the threat model `architecture.md` states — agent error and drift, with hooks as guardrails against the former (principle 14). It is written down so the next reader does not rediscover it, which three readers of three previous claims had to. ## Verified directly rather than taken on the reviewer's word The review terminated early — it moved from reviewing code to probing the Nix fetcher cache and sandbox boundaries, which tripped a provider-side cybersecurity filter — so its report covers the two attacks and nothing else. The remaining questions were checked here: - **No behaviour drift.** `allod-dev`, `nexus`, `privacy-1` and `installer` produce byte-identical toplevel derivation paths across `HEAD~1..HEAD`. The refactor moved code and changed no generated output. - **The other pins still fire.** Restoring a default on `adminAuthorizedKeys` and removing the `homeModules` assertion each turn `service-archetype` red with their own diagnostics. - `./check.sh` passes: all 32 steps. ## Still open, and a judgement call rather than a defect The review was cut off before answering whether the refactor is proportionate. `mkServiceVm` is now the only builder in its own file; `mkDevVm`, `mkPrivacyVm` and `mkHypervisor` remain inline in `flake.nix`, and that inconsistency has a real cost in surprise. The case for it is that this is the only builder whose correctness argument depends on what it *cannot* reach, and `allod/memory` testing.md already records the same shape for `modules/microvm-credential-hook.nix`. The case against is that it buys a boundary which, as above, stops at the call site. Worth a reviewer's opinion before merge; either answer is defensible and it is cheap to inline again.
Author
Member

inventory#14 is merged, so the three follow-ups this PR was holding are now done

3c519a3 bumps the inventory input from 1a1ca7a to 0c41feb and closes the three items the PR body listed as belonging with that bump. ./check.sh passes: all 32 steps.

The lock. The previously pinned revision is the one review confirmed by hash rejects a valid service machine (inventory machines missing runtime: service-1). It now points at the merged service rules, so the archetype is usable rather than merely evaluable.

The duplicated classification is gone. nix/vm-facts.nix kept its own nonGuestTypes = [ "hypervisor" "service" ] while the input predated the export. It now takes isGuestMachine from inventory.lib, which is where machine facts belong (architecture principle 8). This is not tidiness: the hypervisor-only form of exactly this predicate had already gone stale in runtime-module-selection by the time service was added, and a second copy is how that happened. There is now one consumer of one exported fact.

The dependency is load-bearing rather than described. service-archetype now builds inventory.checks.<system>.service-machine-mutations, the same idiom and the same reasoning as runtime-module-selection's dependency on runtime-fact-mutations: whether a service machine may carry a runtime, must be x86_64-linux, and must not carry guest sizing fields is inventory's contract, and this repo cannot re-prove it — those diagnostics are lexical internals there, and a downstream // override lands after inventory's own trip-wire has run. Bound with or (throw …) so an input predating the rules fails with a sentence naming what is missing rather than an attribute error.

Review status

Four rounds across two models. Rounds one to three each found real defects, all fixed and witnessed. Round four broke this PR's central thesis — a two-file edit routing inventory through the lib parameter reads machines while every assertion stays green — which was reproduced here and answered by correcting the claim in all four places it was overstated, rather than by adding a fourth check.

That round was cut short by a provider-side content filter before it reached one question: whether putting mkServiceVm in its own file, while mkDevVm, mkPrivacyVm and mkHypervisor remain inline, is proportionate. That is a judgement call rather than a defect, it is cheap to reverse in either direction, and further review passes are unlikely to settle it. Everything the round did not reach was verified here instead: no behaviour drift (all four machines produce byte-identical toplevel derivations across the refactor), and the adminAuthorizedKeys and homeModules pins both still fire under sabotage.

What this PR does not do

No service machine exists, deliberately. Standing one up still needs an inventory entry, an identity in secrets (machineConfigurations asserts machine names and identity names match exactly), and a profile definition — plus adminAuthorizedKeys, which has no default on purpose. Each of those fails loudly rather than silently. The deploy path itself is allod/archetypes#57.

## inventory#14 is merged, so the three follow-ups this PR was holding are now done `3c519a3` bumps the `inventory` input from `1a1ca7a` to `0c41feb` and closes the three items the PR body listed as belonging with that bump. `./check.sh` passes: all 32 steps. **The lock.** The previously pinned revision is the one review confirmed by hash *rejects* a valid service machine (`inventory machines missing runtime: service-1`). It now points at the merged service rules, so the archetype is usable rather than merely evaluable. **The duplicated classification is gone.** `nix/vm-facts.nix` kept its own `nonGuestTypes = [ "hypervisor" "service" ]` while the input predated the export. It now takes `isGuestMachine` from `inventory.lib`, which is where machine facts belong (architecture principle 8). This is not tidiness: the hypervisor-only form of exactly this predicate had already gone stale in `runtime-module-selection` by the time `service` was added, and a second copy is how that happened. There is now one consumer of one exported fact. **The dependency is load-bearing rather than described.** `service-archetype` now builds `inventory.checks.<system>.service-machine-mutations`, the same idiom and the same reasoning as `runtime-module-selection`'s dependency on `runtime-fact-mutations`: whether a `service` machine may carry a `runtime`, must be `x86_64-linux`, and must not carry guest sizing fields is inventory's contract, and this repo cannot re-prove it — those diagnostics are lexical internals there, and a downstream `//` override lands after inventory's own trip-wire has run. Bound with `or (throw …)` so an input predating the rules fails with a sentence naming what is missing rather than an attribute error. ## Review status Four rounds across two models. Rounds one to three each found real defects, all fixed and witnessed. Round four broke this PR's central thesis — a two-file edit routing inventory through the `lib` parameter reads `machines` while every assertion stays green — which was reproduced here and answered by correcting the claim in all four places it was overstated, rather than by adding a fourth check. That round was cut short by a provider-side content filter before it reached one question: whether putting `mkServiceVm` in its own file, while `mkDevVm`, `mkPrivacyVm` and `mkHypervisor` remain inline, is proportionate. That is a judgement call rather than a defect, it is cheap to reverse in either direction, and further review passes are unlikely to settle it. Everything the round did not reach was verified here instead: no behaviour drift (all four machines produce byte-identical toplevel derivations across the refactor), and the `adminAuthorizedKeys` and `homeModules` pins both still fire under sabotage. ## What this PR does not do No service machine exists, deliberately. Standing one up still needs an inventory entry, an identity in `secrets` (`machineConfigurations` asserts machine names and identity names match exactly), and a profile definition — plus `adminAuthorizedKeys`, which has no default on purpose. Each of those fails loudly rather than silently. The deploy path itself is `allod/archetypes#57`.
vnprc closed this pull request 2026-09-02 03:00:45 +01:00
Author
Member

Closed unmerged — the archetype was the wrong construction, and this repo said so first

Closing this rather than merging it. The work was not wasted: what it produced is the evidence that the service archetype should not exist, and that evidence is worth more than the builder.

The repo predicted this outcome before the work started. The note above builders that this PR deletes says a service builder would "either invent [an inventory concept] now or take every value as an argument, which is a builder in name only." Four review rounds then produced exactly that artifact:

  • platform became a required argument, so the builder stops reading machines;
  • the builder moved into its own file, so inventory is not in lexical scope at all;
  • adminAuthorizedKeys became required, so it stops reaching into secrets.

Every coupling to the framework turned out to be a defect. vmFacts would have aborted on a service machine. runtime-module-selection would have aborted on one. Three separate copies of a guest filter had gone stale. And the default that reached into secrets handed root on a rented, internet-facing box to the nexus host identity — a recipient of every ciphertext in the fleet.

When the only way to make a thing correct is to remove it from its category piece by piece, it was never in the category. Review did not ruin the archetype; review revealed it.

Two further contradictions inside the repo. README.md already documents the merged position: an internet-facing machine "is not in inventory and has no identity in secrets… what this repo gives it is a module its own flake imports." And the profile-definition contract check used the literal string service as its canonical unknown archetype fixture — the tests treated it as a name to reject, and this PR had to change that fixture to make room for itself.

What was actually wrong with the shape. A builder here is the join of inventory × secrets × profiles. A dev guest has all three legs. The hypervisor has all three. A rented static-site host has none: no runtime, no sizing, no MAC, no LAN address, and no identity it needs. A service inventory entry was platform — constrained to a single permitted value — plus the literal string service, with every other field null, empty or meaningless. That is a registry of names, not a model of facts, and a join over no legs is the builder in name only.

What replaces it, and what is kept. Nothing new is needed. nixosModules.publicHost, staticSite and rentedKvmGuest are already exported, already checked standalone — checks/public-host.nix and checks/static-site.nix build their fixtures with nixosSystem directly and never touch a builder, so the library surface was always the tested surface. A rented host is a nixosConfigurations entry composed from those modules in the deploy flake: it stays inside the management path (one lock, one follows discipline, uniform nixos-rebuild --flake) and outside the data-model path (inventory facts, builder dispatch, vmFacts, the identity join).

Issue #55 is not overturned by this. Its axis — what must be carried across a recreation — is the right axis for deciding which module layers exist, and publicHost plus a later statefulService layer remains correct. Its one bridging clause is the part that does not hold: the hypervisor is a non-guest but a full three-legged inventory machine, so "the framework already builds a non-guest machine" does not establish that a rented host needs a builder.

A future static-site VM on nexus is not foreclosed. That machine would have all eleven inventory facts — LAN address, MAC, sizing, runtime, disko layout, identity — and would deserve a builder written then, against a real machine, reusing these same modules. The modules are the shared substance; an archetype is the composition vehicle for machines that actually have inventory facts.

The unknown machine type assertion in machineConfigurations is untouched and will loudly reject a service machine, which is the correct behaviour now.

Refs allod/archetypes#59, allod/archetypes#55

## Closed unmerged — the archetype was the wrong construction, and this repo said so first Closing this rather than merging it. The work was not wasted: what it produced is the evidence that the `service` archetype should not exist, and that evidence is worth more than the builder. **The repo predicted this outcome before the work started.** The note above `builders` that this PR deletes says a service builder would "either invent [an inventory concept] now or take every value as an argument, which is a builder in name only." Four review rounds then produced exactly that artifact: - `platform` became a required argument, so the builder stops reading `machines`; - the builder moved into its own file, so `inventory` is not in lexical scope at all; - `adminAuthorizedKeys` became required, so it stops reaching into `secrets`. Every coupling to the framework turned out to be a defect. `vmFacts` would have aborted on a service machine. `runtime-module-selection` would have aborted on one. Three separate copies of a guest filter had gone stale. And the default that reached into `secrets` handed root on a rented, internet-facing box to the nexus host identity — a recipient of every ciphertext in the fleet. When the only way to make a thing correct is to remove it from its category piece by piece, it was never in the category. Review did not ruin the archetype; review revealed it. **Two further contradictions inside the repo.** `README.md` already documents the merged position: an internet-facing machine "is not in `inventory` and has no identity in `secrets`… what this repo gives it is a module its own flake imports." And the profile-definition contract check used the literal string `service` as its canonical *unknown archetype* fixture — the tests treated it as a name to reject, and this PR had to change that fixture to make room for itself. **What was actually wrong with the shape.** A builder here is the join of inventory × secrets × profiles. A dev guest has all three legs. The hypervisor has all three. A rented static-site host has none: no runtime, no sizing, no MAC, no LAN address, and no identity it needs. A service inventory entry was `platform` — constrained to a single permitted value — plus the literal string `service`, with every other field null, empty or meaningless. That is a registry of names, not a model of facts, and a join over no legs is the builder in name only. **What replaces it, and what is kept.** Nothing new is needed. `nixosModules.publicHost`, `staticSite` and `rentedKvmGuest` are already exported, already checked standalone — `checks/public-host.nix` and `checks/static-site.nix` build their fixtures with `nixosSystem` directly and never touch a builder, so the library surface was always the tested surface. A rented host is a `nixosConfigurations` entry composed from those modules in the deploy flake: it stays inside the management path (one lock, one `follows` discipline, uniform `nixos-rebuild --flake`) and outside the data-model path (inventory facts, builder dispatch, `vmFacts`, the identity join). **Issue #55 is not overturned by this.** Its axis — what must be carried across a recreation — is the right axis for deciding which module *layers* exist, and `publicHost` plus a later `statefulService` layer remains correct. Its one bridging clause is the part that does not hold: the hypervisor is a non-guest but a full three-legged inventory machine, so "the framework already builds a non-guest machine" does not establish that a rented host needs a builder. **A future static-site VM on nexus is not foreclosed.** That machine would have all eleven inventory facts — LAN address, MAC, sizing, runtime, disko layout, identity — and would deserve a builder written then, against a real machine, reusing these same modules. The modules are the shared substance; an archetype is the composition vehicle for machines that actually have inventory facts. The `unknown machine type` assertion in `machineConfigurations` is untouched and will loudly reject a `service` machine, which is the correct behaviour now. Refs allod/archetypes#59, allod/archetypes#55

Pull request closed

Sign in to join this conversation.
No description provided.