fix(resolver): restate the hashed marker from the value a reference resolved - #704
Merged
Merged
Conversation
…esolved Reference resolution rebuilds a consumer's envelope by copying every key off the target and then overwriting $value with what it resolved. When that target came from stored state it carries $hashed:true from an earlier at-rest hash, and the copy preserves it whatever the new value is, so the marker stops describing the value beside it. That breaks in both directions, and they pull opposite ways. A live plaintext resolution inherits a stale true. The claim is what makes it permanent: the terminal-state hashing pass returns early on any envelope already marked hashed, correctly refusing to hash a digest twice, so nothing corrects the mislabelled one. A successful apply hides this, because the completion path replaces the desired properties with hashed read-actual values; when the command fails there is no such replacement and the plaintext persists, labelled as a digest. A resolution that is itself a stored digest needs the opposite. The plugin-boundary guard refuses a write on the $hashed marker alone, so simply clearing the marker would let a digest reach a provider as though it were the secret. So carry the incoming value's hashed-ness through setRefValue and restate the marker from it, rather than inheriting whatever the copy happened to hold. The embedded reference path gets the same treatment: it has the identical copy-then-overwrite shape, though no test here reaches it, so its exposure is unproven and the change is made for the invariant rather than an observed failure.
JeroenSoeters
force-pushed
the
fix/resolution-drops-stale-hashed-marker
branch
from
August 29, 2026 06:02
852622c to
7887f52
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mise ~/.config/mise/config.toml tools: gh@2.98.0
Summary
Reference resolution rebuilds a consumer's envelope by copying every key off the target and then overwriting
$valuewith what it resolved (resolver.go,resolveReference). When that target came from stored state it carries$hashed: truefrom an earlier at-rest hash, and the copy preserves it regardless of the new value, so the marker stops describing the value beside it.That breaks in both directions, and they pull opposite ways.
A live plaintext resolution inherits a stale
true. The claim is what makes it permanent: the terminal-state hashing pass returns early on any envelope already marked hashed, correctly refusing to hash a digest twice, so nothing ever corrects the mislabelled one. A successful apply hides this, because the completion path replaces the desired properties with hashed read-actual values. When the command fails there is no such replacement, and the plaintext persists at rest under a marker claiming it is a digest.A resolution that is itself a stored digest needs the opposite.
guardNoHashedValuesrefuses a provider write on the$hashedmarker alone (scanHashedmatches that key and nothing else), so simply clearing the marker would let a digest reach a provider as though it were the secret.The fix
Carry the incoming value's hashed-ness through
setRefValueonto the resolvedValue, and restate the marker from it rather than inheriting whatever the wholesale copy held. Neither direction then depends on what the target envelope happened to carry.resolveEmbedRefgets the same treatment. It has the identical copy-then-overwrite shape, but no test here reaches it, so its exposure is unproven and the change is made for the invariant rather than for an observed failure. Flagging that so it is reviewed on those terms.Tests
TestApplyForma_DestroyedReferencedSecret_LeavesNoConsumerPlaintextAtRestis the failing-to-passing case. It destroys a secret a consumer still references, so the consumer's update fails and the command ends Failed, then scans every persistent sink for the plaintext. Without the fix the consumer's stored desired state holds an envelope marked"$hashed": truewhose$valueis the literal secret.TestResolvePropertyReferences_HashedMarkerFollowsTheResolvedValuepins both directions. It was checked against a version of this fix that cleared the marker unconditionally: the digest subtest fails there and passes here, so it holds the second direction rather than restating the first.TestApplyForma_FailedCommand_LeavesNoConsumerPlaintextAtRestcovers the adjacent path where a plugin refusal fails the command. It passes without the fix, which is worth knowing: the leak is specific to the destroy path, so a convenient reproduction would have suggested there was nothing to fix.Verification
go test ./internal/workflow_tests/... ./internal/metastructure/... ./internal/datastore/... ./internal/schema/... -tags unit -count=1— exit 0, zero failures, re-run after the final change.Two local environment notes, neither caused by this branch.
go build ./...fails insidego-json-experiment/jsonunder Go 1.27 because the pinned commit conflicts with 1.27's stdlib jsonv2 aliases; the repo'sgo.modsaysgo 1.26.0, soGOTOOLCHAIN=go1.26.0builds and tests cleanly, and CI uses the go.mod toolchain so it never sees this. Andinternal/schema/pkltests are behind//go:build integrationand needmake version-semverin the working tree, or they fail on a missing resource rather than on anything real.