vault_keychain_cleanup.rs writes the biometric enrollment marker with its own temp-plus-rename helper instead of going through fs_util::write_private, so none of what write_private guarantees applies to it. #45 called this "adjacent, not part of this" and #46 left it alone; now that #46 has landed the Windows half, the two implementations have visibly diverged.
write_marker:
let mut options = std::fs::OpenOptions::new();
options.write(true).create(true).truncate(true);
#[cfg(unix)]
{
use std::os::unix::fs::OpenOptionsExt;
options.mode(0o600);
}
let mut file = options.open(&tmp)?;
Three differences from write_private as it now stands:
The marker holds a keychain binding id and a scope token rather than key material, so this is not a disclosure on the scale of vault.json. It is a second implementation of an operation the codebase has already decided how to do, sitting in the same directory as the files that get the careful treatment, and it will keep drifting.
Suggested fix: route the marker through fs_util::write_private. That needs write_private (and a matching read) exposed from clavyn-core rather than pub(crate), which is the only real design question here — desktop is the caller and the marker is desktop-only state. The alternative, duplicating the Windows DACL and owner work in vault_keychain_cleanup.rs, is the outcome worth avoiding.
Not urgent, and deliberately out of scope for #46, which stayed inside core/src/fs_util.rs.
vault_keychain_cleanup.rswrites the biometric enrollment marker with its own temp-plus-rename helper instead of going throughfs_util::write_private, so none of whatwrite_privateguarantees applies to it. #45 called this "adjacent, not part of this" and #46 left it alone; now that #46 has landed the Windows half, the two implementations have visibly diverged.write_marker:Three differences from
write_privateas it now stands:mode(0o600)iscfg(unix)and nothing takes its place, so the marker takes whatever%APPDATA%hands down — the statefs_util.rswas in before Give Windows state files an explicit owner-only DACL instead of the profile's inherited one #46.CREATE_ALWAYS, notCREATE_NEW. A file already sitting atbiometric_enrollment.json.tmpis written through rather than refused, so the file object that ends up under the marker's name can be one another process placed there. On Windows that object carries its owner across the rename, and the owner outranks the DACL. Give Windows state files an explicit owner-only DACL instead of the profile's inherited one #46 fixed exactly this inwrite_private.The marker holds a keychain binding id and a scope token rather than key material, so this is not a disclosure on the scale of
vault.json. It is a second implementation of an operation the codebase has already decided how to do, sitting in the same directory as the files that get the careful treatment, and it will keep drifting.Suggested fix: route the marker through
fs_util::write_private. That needswrite_private(and a matching read) exposed fromclavyn-corerather thanpub(crate), which is the only real design question here —desktopis the caller and the marker is desktop-only state. The alternative, duplicating the Windows DACL and owner work invault_keychain_cleanup.rs, is the outcome worth avoiding.Not urgent, and deliberately out of scope for #46, which stayed inside
core/src/fs_util.rs.