hive-c0re: stop silently dropping errors on credential/marker writes
Add secret_file::write_secret_0600: create the parent dir, write a sibling temp file opened create_new + mode 0600, sync_all, rename over the target. Use it for the matrix sender token, the space/chat room-id files and the forge core token, which previously wrote at the process umask, then chmod-ed with the error discarded, and dropped the create_dir_all error. agent_identity::write_identity uses it too instead of its own open-0600 loop; that loop opened with create + truncate, so a pre-existing file kept its old mode. Also log instead of discarding: - the manager's applied-rev marker write in auto_update - the knowledge `git remote set-url` scrub (spawn error and non-zero exit; the exit status only, since stderr may echo the old URL) - the parent create_dir_all in paths::move_if_legacy Closes #5037
This commit is contained in:
parent
54226e027d
commit
f53ccb2740
8 changed files with 119 additions and 53 deletions
|
|
@ -147,14 +147,9 @@ async fn mint_token(name: &str, scopes: &str) -> Result<String> {
|
||||||
/// by hive-c0re itself (under `/var/lib/hyperhive/`), so a direct
|
/// by hive-c0re itself (under `/var/lib/hyperhive/`), so a direct
|
||||||
/// write is both correct and necessary (no priv round-trip).
|
/// write is both correct and necessary (no priv round-trip).
|
||||||
async fn mint_and_persist_core_token(path: &Path) -> Result<()> {
|
async fn mint_and_persist_core_token(path: &Path) -> Result<()> {
|
||||||
use std::os::unix::fs::PermissionsExt;
|
|
||||||
let token = mint_token("core", CORE_TOKEN_SCOPES).await?;
|
let token = mint_token("core", CORE_TOKEN_SCOPES).await?;
|
||||||
if let Some(parent) = path.parent() {
|
crate::secret_file::write_secret_0600(path, format!("{token}\n").as_bytes())
|
||||||
std::fs::create_dir_all(parent).ok();
|
|
||||||
}
|
|
||||||
std::fs::write(path, format!("{token}\n"))
|
|
||||||
.with_context(|| format!("write core token to {}", path.display()))?;
|
.with_context(|| format!("write core token to {}", path.display()))?;
|
||||||
let _ = std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600));
|
|
||||||
tracing::info!(path = %path.display(), "forge: persisted core access token");
|
tracing::info!(path = %path.display(), "forge: persisted core access token");
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -121,13 +121,12 @@ async fn collect(agent: &str, dir: &Path) -> Result<(), Skipped> {
|
||||||
|
|
||||||
/// Write the two private files, replacing whatever was there.
|
/// Write the two private files, replacing whatever was there.
|
||||||
///
|
///
|
||||||
/// `0600` from the moment each file exists rather than after a `chmod`: the
|
/// `0600` from the moment each file exists rather than after a `chmod`
|
||||||
/// key is written through a handle opened with that mode, so there is no
|
/// ([`crate::secret_file::write_secret_0600`]), so there is no instant where
|
||||||
/// instant where it sits on disk world-readable.
|
/// the key sits on disk world-readable.
|
||||||
fn write_identity(dir: &Path, credential: &mtls::Credential) -> anyhow::Result<()> {
|
fn write_identity(dir: &Path, credential: &mtls::Credential) -> anyhow::Result<()> {
|
||||||
use anyhow::Context as _;
|
use anyhow::Context as _;
|
||||||
use std::io::Write as _;
|
use std::os::unix::fs::PermissionsExt as _;
|
||||||
use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _};
|
|
||||||
|
|
||||||
std::fs::create_dir_all(dir).with_context(|| format!("create {}", dir.display()))?;
|
std::fs::create_dir_all(dir).with_context(|| format!("create {}", dir.display()))?;
|
||||||
// The directory too: its own mode is what stops anything but `hive-core`
|
// The directory too: its own mode is what stops anything but `hive-core`
|
||||||
|
|
@ -139,19 +138,7 @@ fn write_identity(dir: &Path, credential: &mtls::Credential) -> anyhow::Result<(
|
||||||
(CERT_CREDENTIAL, credential.cert.as_bytes()),
|
(CERT_CREDENTIAL, credential.cert.as_bytes()),
|
||||||
(KEY_CREDENTIAL, credential.key.as_bytes()),
|
(KEY_CREDENTIAL, credential.key.as_bytes()),
|
||||||
] {
|
] {
|
||||||
let file = dir.join(name);
|
crate::secret_file::write_secret_0600(&dir.join(name), bytes)?;
|
||||||
let mut handle = std::fs::OpenOptions::new()
|
|
||||||
.write(true)
|
|
||||||
.create(true)
|
|
||||||
.truncate(true)
|
|
||||||
.mode(0o600)
|
|
||||||
.open(&file)
|
|
||||||
.with_context(|| format!("open {}", file.display()))?;
|
|
||||||
// The error carries the path and never the bytes — the one line in
|
|
||||||
// this function where a value could otherwise be interpolated.
|
|
||||||
handle
|
|
||||||
.write_all(bytes)
|
|
||||||
.with_context(|| format!("write {}", file.display()))?;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Public material, and `0644` says so — it is the same bundle every host
|
// Public material, and `0644` says so — it is the same bundle every host
|
||||||
|
|
|
||||||
|
|
@ -29,6 +29,7 @@ mod meta;
|
||||||
mod migrate;
|
mod migrate;
|
||||||
mod paths;
|
mod paths;
|
||||||
mod priv_client;
|
mod priv_client;
|
||||||
|
mod secret_file;
|
||||||
mod server;
|
mod server;
|
||||||
mod snapshot_push;
|
mod snapshot_push;
|
||||||
mod socket_server;
|
mod socket_server;
|
||||||
|
|
|
||||||
|
|
@ -194,14 +194,8 @@ fn sender_source<'a>(stored: Option<&'a str>, on_disk: Option<&str>) -> SenderSo
|
||||||
/// directory if it is not there. The file's mode is the only thing keeping an
|
/// directory if it is not there. The file's mode is the only thing keeping an
|
||||||
/// unprivileged reader off the hive's matrix credential.
|
/// unprivileged reader off the hive's matrix credential.
|
||||||
fn persist_sender_token(path: &std::path::Path, access_token: &str) -> Result<()> {
|
fn persist_sender_token(path: &std::path::Path, access_token: &str) -> Result<()> {
|
||||||
use std::os::unix::fs::PermissionsExt;
|
crate::secret_file::write_secret_0600(path, format!("{access_token}\n").as_bytes())
|
||||||
|
|
||||||
if let Some(parent) = path.parent() {
|
|
||||||
std::fs::create_dir_all(parent).ok();
|
|
||||||
}
|
|
||||||
std::fs::write(path, format!("{access_token}\n"))
|
|
||||||
.with_context(|| format!("matrix: write the sender token to {}", path.display()))?;
|
.with_context(|| format!("matrix: write the sender token to {}", path.display()))?;
|
||||||
let _ = std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600));
|
|
||||||
tracing::info!(path = %path.display(), "matrix: provisioned the sender token");
|
tracing::info!(path = %path.display(), "matrix: provisioned the sender token");
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
@ -308,15 +302,9 @@ pub fn read_sender_token() -> Result<String> {
|
||||||
|
|
||||||
/// Persist the hive Space room id to [`hive_space_room_id_path()`] (0600).
|
/// Persist the hive Space room id to [`hive_space_room_id_path()`] (0600).
|
||||||
fn persist_space_room_id(room_id: &str) -> Result<()> {
|
fn persist_space_room_id(room_id: &str) -> Result<()> {
|
||||||
use std::os::unix::fs::PermissionsExt;
|
|
||||||
let path = hive_space_room_id_path();
|
let path = hive_space_room_id_path();
|
||||||
if let Some(parent) = path.parent() {
|
crate::secret_file::write_secret_0600(&path, format!("{room_id}\n").as_bytes())
|
||||||
std::fs::create_dir_all(parent).ok();
|
.with_context(|| format!("matrix: write space room_id to {}", path.display()))
|
||||||
}
|
|
||||||
std::fs::write(&path, format!("{room_id}\n"))
|
|
||||||
.with_context(|| format!("matrix: write space room_id to {}", path.display()))?;
|
|
||||||
let _ = std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600));
|
|
||||||
Ok(())
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Scan the admin account's joined rooms for the canonical hive Space: the
|
/// Scan the admin account's joined rooms for the canonical hive Space: the
|
||||||
|
|
@ -449,15 +437,9 @@ pub async fn ensure_hive_space(client: &reqwest::Client, sender_token: &str) ->
|
||||||
|
|
||||||
/// Persist the hive chat room id to [`hive_chat_room_id_path()`] (0600).
|
/// Persist the hive chat room id to [`hive_chat_room_id_path()`] (0600).
|
||||||
fn persist_chat_room_id(room_id: &str) -> Result<()> {
|
fn persist_chat_room_id(room_id: &str) -> Result<()> {
|
||||||
use std::os::unix::fs::PermissionsExt;
|
|
||||||
let path = hive_chat_room_id_path();
|
let path = hive_chat_room_id_path();
|
||||||
if let Some(parent) = path.parent() {
|
crate::secret_file::write_secret_0600(&path, format!("{room_id}\n").as_bytes())
|
||||||
std::fs::create_dir_all(parent).ok();
|
.with_context(|| format!("matrix: write chat room_id to {}", path.display()))
|
||||||
}
|
|
||||||
std::fs::write(&path, format!("{room_id}\n"))
|
|
||||||
.with_context(|| format!("matrix: write chat room_id to {}", path.display()))?;
|
|
||||||
let _ = std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600));
|
|
||||||
Ok(())
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Scan the admin account's joined rooms for the canonical hive chat room:
|
/// Scan the admin account's joined rooms for the canonical hive chat room:
|
||||||
|
|
|
||||||
|
|
@ -295,8 +295,14 @@ fn move_if_legacy(old: &Path, new: &Path) {
|
||||||
if !old.exists() || new.exists() {
|
if !old.exists() || new.exists() {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if let Some(parent) = new.parent() {
|
if let Some(parent) = new.parent()
|
||||||
let _ = std::fs::create_dir_all(parent);
|
&& let Err(e) = std::fs::create_dir_all(parent)
|
||||||
|
{
|
||||||
|
tracing::warn!(
|
||||||
|
dir = %parent.display(),
|
||||||
|
error = ?e,
|
||||||
|
"relocate: create parent dir failed"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
match std::fs::rename(old, new) {
|
match std::fs::rename(old, new) {
|
||||||
Ok(()) => tracing::info!(
|
Ok(()) => tracing::info!(
|
||||||
|
|
|
||||||
79
hive-c0re/src/secret_file.rs
Normal file
79
hive-c0re/src/secret_file.rs
Normal file
|
|
@ -0,0 +1,79 @@
|
||||||
|
//! [`write_secret_0600`]: the one way hive-c0re puts a credential on disk.
|
||||||
|
|
||||||
|
use std::io::Write as _;
|
||||||
|
use std::os::unix::fs::OpenOptionsExt as _;
|
||||||
|
use std::path::Path;
|
||||||
|
|
||||||
|
use anyhow::{Context as _, Result};
|
||||||
|
|
||||||
|
/// Write `contents` to `path` as a `0600` file, creating the parent directory.
|
||||||
|
///
|
||||||
|
/// The bytes go to a sibling temp file opened `0600` and are renamed over
|
||||||
|
/// `path`, so the file is never readable by anyone else for an instant, and a
|
||||||
|
/// reader (or a crash) never sees half of it. Errors carry paths, never
|
||||||
|
/// `contents`.
|
||||||
|
pub fn write_secret_0600(path: &Path, contents: &[u8]) -> Result<()> {
|
||||||
|
if let Some(parent) = path.parent() {
|
||||||
|
std::fs::create_dir_all(parent).with_context(|| format!("create {}", parent.display()))?;
|
||||||
|
}
|
||||||
|
let mut tmp_name = path
|
||||||
|
.file_name()
|
||||||
|
.with_context(|| format!("{} has no file name", path.display()))?
|
||||||
|
.to_owned();
|
||||||
|
tmp_name.push(".tmp");
|
||||||
|
let tmp = path.with_file_name(tmp_name);
|
||||||
|
// A temp file left by a crashed earlier write would fail `create_new`.
|
||||||
|
let _ = std::fs::remove_file(&tmp);
|
||||||
|
let mut f = std::fs::OpenOptions::new()
|
||||||
|
.write(true)
|
||||||
|
.create_new(true)
|
||||||
|
.mode(0o600)
|
||||||
|
.open(&tmp)
|
||||||
|
.with_context(|| format!("create {}", tmp.display()))?;
|
||||||
|
f.write_all(contents)
|
||||||
|
.and_then(|()| f.sync_all())
|
||||||
|
.with_context(|| format!("write {}", tmp.display()))?;
|
||||||
|
std::fs::rename(&tmp, path).with_context(|| format!("rename onto {}", path.display()))
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::write_secret_0600;
|
||||||
|
use std::os::unix::fs::PermissionsExt as _;
|
||||||
|
|
||||||
|
fn mode(path: &std::path::Path) -> u32 {
|
||||||
|
std::fs::metadata(path).expect("stat").permissions().mode() & 0o777
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn creates_parent_and_writes_0600() {
|
||||||
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
|
let path = dir.path().join("sub").join("token");
|
||||||
|
write_secret_0600(&path, b"secret\n").expect("writes");
|
||||||
|
assert_eq!(std::fs::read(&path).expect("read"), b"secret\n");
|
||||||
|
assert_eq!(mode(&path), 0o600);
|
||||||
|
assert!(!dir.path().join("sub").join("token.tmp").exists());
|
||||||
|
}
|
||||||
|
|
||||||
|
/// An existing file at a looser mode is replaced, not rewritten in place:
|
||||||
|
/// the new file is 0600 and holds only the new bytes.
|
||||||
|
#[test]
|
||||||
|
fn replaces_an_existing_looser_file() {
|
||||||
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
|
let path = dir.path().join("token");
|
||||||
|
std::fs::write(&path, "a much longer old value\n").expect("seed");
|
||||||
|
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).expect("chmod");
|
||||||
|
write_secret_0600(&path, b"new\n").expect("writes");
|
||||||
|
assert_eq!(std::fs::read(&path).expect("read"), b"new\n");
|
||||||
|
assert_eq!(mode(&path), 0o600);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_stale_temp_file_does_not_block_the_write() {
|
||||||
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
|
let path = dir.path().join("token");
|
||||||
|
std::fs::write(dir.path().join("token.tmp"), "stale").expect("seed");
|
||||||
|
write_secret_0600(&path, b"new\n").expect("writes");
|
||||||
|
assert_eq!(std::fs::read(&path).expect("read"), b"new\n");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -208,8 +208,10 @@ pub async fn ensure_root_agent(coord: &Arc<Coordinator>) -> Result<()> {
|
||||||
if let Err(e) = coord.power.set(MANAGER_NAME, crate::power::Wanted::Up) {
|
if let Err(e) = coord.power.set(MANAGER_NAME, crate::power::Wanted::Up) {
|
||||||
tracing::warn!(error = ?e, "agent_power: set manager wanted=up failed");
|
tracing::warn!(error = ?e, "agent_power: set manager wanted=up failed");
|
||||||
}
|
}
|
||||||
if let Some(rev) = current_rev {
|
if let Some(rev) = current_rev
|
||||||
let _ = std::fs::write(crate::paths::applied_rev_marker(MANAGER_NAME), &rev);
|
&& let Err(e) = std::fs::write(crate::paths::applied_rev_marker(MANAGER_NAME), &rev)
|
||||||
|
{
|
||||||
|
tracing::warn!(name = MANAGER_NAME, error = ?e, "write rev marker failed");
|
||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -109,10 +109,24 @@ pub async fn pull(coord: &Coordinator) -> Result<()> {
|
||||||
// place this repo stored a token in a named remote. Harmless to repeat
|
// place this repo stored a token in a named remote. Harmless to repeat
|
||||||
// once the URL is already clean.
|
// once the URL is already clean.
|
||||||
let plain = crate::forge::forge_git_url(&format!("{ORG}/{REPO}"));
|
let plain = crate::forge::forge_git_url(&format!("{ORG}/{REPO}"));
|
||||||
let _ = crate::lifecycle::git_command()
|
// Logs the exit status only: git's stderr may echo the old, credentialed URL.
|
||||||
|
match crate::lifecycle::git_command()
|
||||||
.args(["-C", LOCAL_DIR, "remote", "set-url", "origin", &plain])
|
.args(["-C", LOCAL_DIR, "remote", "set-url", "origin", &plain])
|
||||||
.output()
|
.output()
|
||||||
.await;
|
.await
|
||||||
|
{
|
||||||
|
Ok(out) if out.status.success() => {}
|
||||||
|
Ok(out) => tracing::warn!(
|
||||||
|
remote = "origin",
|
||||||
|
status = %out.status,
|
||||||
|
"knowledge: git remote set-url failed; a credentialed origin URL may remain"
|
||||||
|
),
|
||||||
|
Err(e) => tracing::warn!(
|
||||||
|
remote = "origin",
|
||||||
|
error = ?e,
|
||||||
|
"knowledge: spawning git remote set-url failed"
|
||||||
|
),
|
||||||
|
}
|
||||||
// Discard any local drift before pulling — both tracked (`reset --hard`)
|
// Discard any local drift before pulling — both tracked (`reset --hard`)
|
||||||
// and untracked (`clean -fd`). Nothing in this codebase writes to
|
// and untracked (`clean -fd`). Nothing in this codebase writes to
|
||||||
// `LOCAL_DIR` after the initial clone — this working tree exists to mirror
|
// `LOCAL_DIR` after the initial clone — this working tree exists to mirror
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue