contracts: setKmsQuote and setKmsEventlog are the only owner mutations that emit no event, so the published KMS evidence can be rewritten silently

[kvinwang]

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

What the design currently is

DstackKms declares PolicyChanged as the audit primitive:

/// @notice Additive audit event for reconstructing authorization policy.
/// @dev `value` is the affected bytes32 value, address, or hash of dynamic public data.
event PolicyChanged(address indexed actor, bytes32 indexed policy, bytes32 indexed value, bool enabled);

Every owner mutation on both contracts emits one — setKmsInfo, setGatewayAppId, setAppImplementation, registerApp, the six add/remove pairs, _authorizeUpgrade, and on DstackApp the compose-hash, device, flag and disableUpgrades setters. test/EventAudit.t.sol pins the reconstruction property for all of them.

Two functions emit nothing at all:

// contracts/DstackKms.sol:119
function setKmsQuote(bytes memory quote) external onlyOwner {
    kmsInfo.quote = quote;
}

// contracts/DstackKms.sol:124
function setKmsEventlog(bytes memory eventlog) external onlyOwner {
    kmsInfo.eventlog = eventlog;
}

No typed event, no PolicyChanged. Their sibling setKmsInfo — which writes these same two fields — emits both KmsInfoSet and PolicyChanged("kms-info", keccak256(k256Pubkey), true), five lines above.

[PASS] test_S6_SetKmsQuoteAndEventlogMutateStateSilently() (gas: 132992)
       assertEq(logs.length, 0, "two owner state mutations, zero log entries");
       assertEq(quote,    hex"deadbeef");
       assertEq(eventlog, hex"cafebabe");

Steelman

kmsInfo.quote and kmsInfo.eventlog are not authorization policy in the narrow sense — no isAppAllowed or isKmsAllowed branch reads them — so a literal reading of the event's docstring excludes them. They are also the two largest fields in the struct: a TDX quote is several kilobytes and an event log can be larger, so emitting them (or even hashing them) on every write has a real gas cost that setKmsInfo's existing keccak256(info.k256Pubkey) choice deliberately avoids by hashing only the small field. These two setters exist precisely so an operator can replace bulky evidence without rewriting the whole struct, and adding a hash-and-emit re-imposes part of the cost they were added to avoid.

docs/specification.md §3.7 lists both under the uniform owner-only mutation pattern and does not claim they emit anything, so the spec and the code agree today.

What it costs

kmsInfo is the published attestation evidence. It is what a relying party fetches to verify that the KMS root key it is trusting came out of a genuine TEE running a known measurement. A silent rewrite of the quote or the event log changes what every future verifier checks against, and leaves no on-chain trace that it happened or who did it.

That makes it the one thing the audit event most obviously exists to cover, even though it is not a branch in a decision function. The narrow reading of "authorization policy" is the reading under which the gap is defensible; the reading an auditor will apply — "can I reconstruct, from logs alone, every owner action that affects what a relying party will accept?" — is the one it fails.

It is also the clearest instance of the "right pattern next door" shape in this codebase: two functions, five lines apart, writing the same storage, one emitting two events and one emitting none.

Reachability: who — the KMS owner; credential — the KMS owner key; frequency — operator-paced. Not attacker-triggered. The cost is to after-the-fact auditability, not to live authorization.

Improvement direction

Redeployment status: implementation upgrade behind the existing DstackKms proxy, no storage-layout change. Two lines. There is a single DstackKms proxy per deployment under the platform owner's control, so one upgrade covers everything — this is about as cheap as a contract fix gets, which is what should decide its priority relative to the more structural items.

Options:

1. Emit PolicyChanged with the hash of the new value (impl upgrade, no storage change). _emitPolicy("kms-quote", keccak256(quote), true) and _emitPolicy("kms-eventlog", keccak256(eventlog), true). Costs one keccak256 over the blob plus one log per call, on a function that is already writing that blob to storage — the hash is a rounding error next to the SSTOREs. Gives a replayer the actor, the timestamp and a commitment to exactly which bytes were installed. Recommended.
2. Emit a bare marker instead (impl upgrade, no storage change). _emitPolicy("kms-quote", bytes32(0), true) — records that a change happened and by whom, without the hashing cost. Weaker: a replayer learns that the evidence changed but not to what, so it cannot detect a rewrite-and-restore.
3. Add typed events — KmsQuoteSet(bytes32 quoteHash) / KmsEventlogSet(bytes32 eventlogHash) — alongside or instead of (1), matching the KmsInfoSet precedent. Mostly a style question; PolicyChanged alone is sufficient for the reconstruction property and a typed event is friendlier to existing indexers.
4. Remove the two setters and require a full setKmsInfo (impl upgrade, no storage change). Eliminates the divergence by eliminating the sibling. Breaks any tooling that calls them and re-imposes the gas cost they exist to avoid; listed for completeness.
5. Spec + test, independent of the above. Extend docs/specification.md §3.7 to state the event obligation explicitly for every owner mutation, and add these two to test/EventAudit.t.sol so the property is pinned rather than assumed. Worth doing whichever option is chosen — the test is what stops the next setter from being added without one.

(1) plus (5).

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