contracts: the three DstackKms allowlist setters still accept bytes32(0), and #1268's OS-image fix depends on allowedOsImages[0] being false

Start with the DstackKms implementation and dstack/kms/auth-eth/test/ScenarioWalk.t.sol, then read scenario 4 in .agent/CONTRACT-SCENARIOS.md and compare the three setters with contracts/DstackApp.sol's existing guards. Run the scenario test and review Manage.s.sol; done means the chosen zero-value handling and preflight behavior are covered without a storage-layout change.

Label: DESIGN. No test fails against the current contracts; the test below passes and pins present behaviour.

What the design currently is

PR #1268 added a zero guard to the two DstackApp allowlist setters, with the reasoning stated in the diff:

// contracts/DstackApp.sol, added by #1268
// Both Ethereum auth backends left-pad a short hex value to the full width
// before calling in, so bytes32(0) is what an absent or truncated
// composeHash/deviceId arrives as. `_initializeCommon` already declines to
// seed either from zero; the setters used to disagree.
require(composeHash != bytes32(0), "invalid compose hash");
require(deviceId   != bytes32(0), "invalid device ID");

The three DstackKms siblings did not get the same guard. addOsImageHash, addKmsAggregatedMr and addKmsDevice all accept bytes32(0):

[PASS] test_S4_EveryAdderAcceptsBytes32Zero_WhileTheInitializerRefusesIt() (gas: 354847)
       kms.addOsImageHash(bytes32(0));      // succeeds
       kms.addKmsAggregatedMr(bytes32(0));  // succeeds
       kms.addKmsDevice(bytes32(0));        // succeeds
       assertTrue(ok, "and the KMS gate too");   // all-zero boot info passes isKmsAllowed

The reasoning #1268 gives applies verbatim to all three: both Ethereum backends padStart a short value to full width (auth-eth-bun/index.ts decodeHex, auth-eth/src/ethereum.ts decodeHex), so an absent field arrives as bytes32(0).

This is not hypothetical for osImageHash in particular. An empty os_image_hash is a documented, reachable condition on the KMS onboarding path — ensure_kms_allowed has an explicit if boot_info.os_image_hash.is_empty() branch for legacy-format sources (kms/src/main_service/upgrade_authority.rs:216-243).

And #1268's own K-c fix depends on the invariant this gap leaves unenforced. From that PR's description:

narrow the fallback to attestations carrying no config at all — a source that presents a config and still reports no image now keeps an empty hash and is denied, since allowedOsImages[bytes32(0)] is false.

That "since" is an assumption about KMS contract state, not a property the KMS contract guarantees. One cast send $KMS "addOsImageHash(bytes32)" $OS_IMAGE_HASH with OS_IMAGE_HASH unset — which expands to the empty string and is a routine shell footgun — makes it true and silently re-opens the path #1268 closed. (The Manage.s.sol:AddOsImage script uses vm.envBytes32, which does fail on an unset variable; the cast send form the tutorials show does not.)

Steelman

The DstackKms values are owner-controlled, and the trust model (docs/specification.md §1) trusts the KMS owner for all write operations, so a guard against the owner's own typo is arguably out of scope. The DstackApp case #1268 fixed had a sharper argument behind it — app owners are a much larger and less operationally sophisticated set than KMS owners, and the compose-hash padding attack had a concrete path. Adding three requires to a contract that is already deployed also costs an implementation upgrade and a redeployment review cycle for a defence-in-depth measure.

It is also true that each of these three values is AND-composed with the others in isKmsAllowed, so no single zero entry is sufficient on its own.

What it costs

It costs the DstackKms half of a defence #1268 already decided was worth having, and it leaves a stated fix resting on an unenforced precondition. The generalisation the sibling argument makes — "zero is the padded form of an absent value, and the initializer already refuses it" — is true of all five setters; three of them still disagree with it.

Concretely, in the retire-an-OS-image scenario: allowedOsImages[bytes32(0)] == true means any boot presenting an absent or unverifiable OS image hash passes the image gate. Combined with image.verify = false (K-h in AUDIT-BACKLOG.md, where BootInfo.os_image_hash becomes whatever the caller put in vm_config), the image allowlist degrades to a check on an attacker-supplied value that the contract has been told to accept.

Reachability: who — the KMS owner; credential — the KMS owner key; frequency — one mistyped transaction, and it is persistent once made. Not attacker-triggered; this is a guard against an irreversible operator error whose effect is silent.

Improvement direction

Redeployment status: implementation upgrade behind the existing DstackKms proxy, no storage-layout change. Three require lines. Unlike the DstackApp case, there is a single DstackKms proxy per deployment under the platform owner's control, so the fix reaches every app as soon as that one upgrade lands — this is materially cheaper to roll out than #1268's DstackApp half.

Options:

1. Mirror #1268 exactly (impl upgrade, no storage change). require(osImageHash != bytes32(0)), require(mrAggregated != bytes32(0)), require(deviceId != bytes32(0)) in the three adders, with the same comment. Blocks only new zero entries, so it is compatible with any existing state. Recommended.
2. Also guard the read side (impl upgrade, no storage change). Short-circuit isKmsAllowed/isAppAllowed on a zero osImageHash / mrAggregated / deviceId regardless of the mapping. Stronger — it also neutralises a zero entry added before the upgrade — at the cost of a few gas on the happy path and a behaviour change if any deployment is relying on a zero entry today (none should be, but that needs checking against live state first).
3. Off-chain only, no contract change. Have the KMS refuse to present a zero-valued identity field, extending #1268's ensure_identity_widths. Cheapest to ship and reaches deployments that cannot upgrade their KMS contract, but leaves the contract state itself misconfigured and does nothing for any other consumer of isKmsAllowed.
4. Documentation + a preflight check in Manage.s.sol. Independent of the above and worth doing regardless: the BatchKmsSetup loop reads vm.envOr(..., new bytes32[](0)) and would happily add a zero element from a malformed list.

(1) plus (4) is the minimal honest fix; (2) if the team wants the read side closed against pre-existing state.

Found during a scenario-driven review of the authorization contracts; full walk in .agent/CONTRACT-SCENARIOS.md (scenario 4), test in dstack/kms/auth-eth/test/ScenarioWalk.t.sol.