Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions hyperdb-mcp/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
`bar_orientation`, `label_values`, `show_legend`, and positive-only
`y_scale`, while preserving the public Rust `ChartOptions` surface and
existing rendering defaults.
- **`daemon::state_perms` module, with `state_perms::ensure_owner_only_dir`.**
New public surface on the library target: the single place that decides how
the daemon's state directory and the files in it are created and tightened
(see Security, below). It is public because `hyperdb-mcp`'s `[[bin]]` is a
separate crate to Cargo and sets up the daemon's log directory itself;
`pub(crate)` would not reach it. Like the rest of `daemon::*` it is plumbing
for the binary rather than an API to build on — the library target "is not a
documented API surface" — so it may be narrowed without a breaking change.

### Changed

Expand Down Expand Up @@ -280,6 +288,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/).

### Fixed

- **On Windows, the daemon state directory now resolves from `%USERPROFILE%`
before `HOME`.** `discovery::state_dir` documented `~` as "`HOME` on Unix,
`USERPROFILE` on Windows" but `home_dir()` tried `HOME` first on every
platform. MSYS2, Cygwin and Git Bash routinely set `HOME` to a path of their
own outside the user profile, so under those shells the state directory
landed outside `%USERPROFILE%` and did not inherit its ACL — the only thing
restricting these files on Windows — without the user having set
`HYPERDB_STATE_DIR` or otherwise asked for it. Windows now prefers
`USERPROFILE`, keeps `HOME` as a last resort so a machine that resolved
before still resolves, and matches `paths::persistent_home_dir`. Unix
resolution is unchanged. **Behaviour change on Windows:** a shell that sets
both to different paths now gets the profile-relative state directory, so a
daemon started before this change may not be discovered by a client started
after it until the old one is stopped.
- **Daemon port `0` is now rejected at both entry points, instead of quietly
multiplying daemons.** `--port` promises an exact bind and
`"0".parse::<u16>()` succeeds, so `0` passed validation at both the flag and
Expand Down Expand Up @@ -632,6 +654,50 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
the file has not committed. Fixes
[issue #284](https://github.com/tableau/hyper-api-rust/issues/284).

### Security

- **Daemon state files are now restricted to the owning user.** The state
directory (`~/.hyperdb`, or `HYPERDB_STATE_DIR`) is created `0700` and
`daemon.json` `0600` on Unix, where previously both took their mode from the
process umask — commonly `0755` and `0644`. `daemon.json` names the `hyperd`
endpoint, so it is owner-only from the moment it exists: the mode is set on
the atomic write's temp file *before* any content is written, and the
subsequent `rename` replaces the target's inode, which also tightens a record
an earlier release left readable. `logs/` gets the same `0700` treatment,
since `hyperd` writes its own diagnostic logs there under its own umask and
those records name the endpoint too — restricting the directory covers a
file this process does not own and has not seen yet. Directories and the
regular files directly inside them are both corrected when an earlier run
left them loose, rather than accepted as-is: closing the directory does
nothing about a log file already in it, and `logs/` is where the daemon's own
log and `hyperd`'s rotated logs sit. The sweep is one level deep, skips
anything that is not a regular file, and warns rather than failing, so it
cannot follow a link out of the directory or walk into whatever
`HYPERDB_STATE_DIR` names.
- **The discovery file's temp file is created exclusively, so the mode always
applies to a file of our own.** `write_discovery_record` wrote
`daemon.json.tmp` with a plain create, which opens whatever already bears
that name and leaves the intended mode dependent on what that turns out to
be — a leftover temp file kept its own mode, and a name that resolved
somewhere else was written through. It is now unlinked and recreated with
`O_CREAT | O_EXCL` and `O_NOFOLLOW`, so the record always lands on a regular
file inside the state directory, created with the mode it is meant to have;
if the name cannot be created cleanly the write fails rather than publishing
anyway.
- **A filesystem that cannot represent Unix modes no longer costs the daemon
its discovery file.** `chmod` is refused outright on a mount whose
permissions come from `fmask`/`dmask` rather than from each file — SMB or
NFS, exFAT or vfat, some container bind-mounts. Tightening the *directory*
already warned and carried on there, but setting the mode on `daemon.json`
propagated the refusal, so on exactly those filesystems the daemon warned
about the directory and then could not publish a record at all — turning a
working-if-loose setup into a startup failure. A refusal is now reconciled
against the mode actually on disk: if the file already reads back with
nothing granted to group or other, the record is protected and is published;
if it reads back wider, the error stands and nothing is published. Windows
relies on the ACL that `%USERPROFILE%` subdirectories inherit, which already
excludes other interactive users.

## [0.5.0] - 2026-06-07

### Added
Expand Down
6 changes: 5 additions & 1 deletion hyperdb-mcp/DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,11 @@ Logs land next to the persistent file when one is supplied (so users find them i
`Engine::new` defaults to *daemon mode* — it tries
`daemon::spawn::ensure_daemon(resolve_port_scan())` first, which discovers an
existing daemon via `~/.hyperdb/daemon.json` (overridable via
`HYPERDB_STATE_DIR`), else scans the port range for a running daemon, else
`HYPERDB_STATE_DIR` — see `daemon::state_perms`, which restricts that directory
and the files in it to the owning user, and which needs the override to name a
path that can carry those permissions: inside `%USERPROFILE%` on Windows, on a
mode-supporting filesystem on Unix), else scans the port range for a running
daemon, else
auto-spawns one on the first free port as a detached background process. The
Engine then connects via TCP (`Connection::connect(endpoint, …)`) without
owning any `HyperProcess`, and records the daemon's `health_port` so the
Expand Down
13 changes: 12 additions & 1 deletion hyperdb-mcp/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,15 @@ hyperdb-mcp daemon # Run as a daemon explicitly (rarely needed)
`status` and `stop` locate the running daemon automatically (reading `daemon.json`, then scanning the port range), so they work even if the daemon scanned onto a non-default port. Pass `--port <PORT>` to target a specific port explicitly.

State files live at `~/.hyperdb/` by default (override with `HYPERDB_STATE_DIR`).
They record the `hyperd` endpoint, so the daemon restricts them to your own
account: `0700` on the directories and `0600` on `daemon.json` on Unix, and on
Windows the ACL a `%USERPROFILE%` subdirectory inherits. If you override the
location, keep it somewhere that can carry those permissions — under your user
profile on Windows, and on a filesystem that supports Unix modes on Unix (a
network share or a FAT/exFAT volume takes its modes from mount options
instead). The daemon warns rather than refusing to start when it cannot tighten
the directory, but it will not publish `daemon.json` into a file it cannot keep
readable by you alone.

For installation and configuration diagnostics that also work before MCP can start, use the native doctor command:

Expand Down Expand Up @@ -970,7 +979,9 @@ Environment:
HYPERD_PATH Hyperd executable or containing directory; when absent or
non-UTF-8, walk upward for .hyperd/current/hyperd (no PATH lookup)
HYPERDB_PERSISTENT_DB Override the default persistent-db path
HYPERDB_STATE_DIR Override daemon state directory (default ~/.hyperdb/)
HYPERDB_STATE_DIR Override daemon state directory (default ~/.hyperdb/); keep it
under your user profile on Windows and on a filesystem with Unix
modes on Unix, or it cannot be restricted to your account
HYPERDB_DAEMON_PORT Pin auto-spawn discovery to one health/lock candidate;
foreground startup binds this configured/base port exactly
HYPERDB_DAEMON_IDLE_TIMEOUT Opt into idle shutdown (seconds); default: stay resident
Expand Down
135 changes: 127 additions & 8 deletions hyperdb-mcp/src/daemon/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,22 +296,25 @@ pub(super) fn write_enriched_discovery_file(info: &DaemonInfo) -> io::Result<()>

fn write_discovery_record(record: &(impl Serialize + ?Sized)) -> io::Result<()> {
let dir = state_dir()?;
std::fs::create_dir_all(&dir)?;
super::state_perms::ensure_owner_only_dir(&dir)?;

let path = dir.join("daemon.json");
let tmp_path = dir.join("daemon.json.tmp");
let json = serde_json::to_string_pretty(record).map_err(|e| io::Error::other(e.to_string()))?;
std::fs::write(&tmp_path, json.as_bytes())?;
// `std::fs::rename` already replaces an existing target atomically on
// both Unix (`rename(2)`) and Windows (`MoveFileExW` with
// Writes into `tmp_path` and renames it onto `path`. `std::fs::rename`
// already replaces an existing target atomically on both Unix
// (`rename(2)`) and Windows (`MoveFileExW` with
// `MOVEFILE_REPLACE_EXISTING`, falling back to `SetFileInformationByHandle`
// with `FILE_RENAME_FLAG_REPLACE_IF_EXISTS`). Pre-deleting the target here
// with `FILE_RENAME_FLAG_REPLACE_IF_EXISTS`). Pre-deleting the target
// would reintroduce exactly the window this function's doc comment
// promises not to have: a concurrent `discover()` could observe the file
// as `Missing` mid-restart (see `try_restart_hyperd`, which rewrites this
// file on every `hyperd` restart).
std::fs::rename(&tmp_path, &path)?;
Ok(())
//
// The rename is also what tightens a record an earlier release left
// world-readable, because it replaces the target's inode rather than
// rewriting it in place.
super::state_perms::write_owner_only_atomic(&path, &tmp_path, json.as_bytes())
}

/// Read the discovery file and validate that the daemon is still alive.
Expand Down Expand Up @@ -451,8 +454,22 @@ pub fn resolve_port() -> u16 {
}

/// Cross-platform home directory resolution.
///
/// On Windows the user profile is consulted first. MSYS2, Cygwin and Git Bash
/// commonly set `HOME` to a path of their own outside `%USERPROFILE%`, and
/// preferring it would put the state directory outside the profile — losing the
/// inherited ACL that is the whole of the Windows protection for these files
/// (see [`super::state_perms`]) without the user having asked for it. `HOME`
/// stays as a last resort there, so a machine that resolved before still
/// resolves. This is also the order the documentation above already described,
/// and it matches `crate::paths::persistent_home_dir`.
fn home_dir() -> Option<PathBuf> {
// Try HOME (Unix) then USERPROFILE (Windows)
if cfg!(windows) {
return std::env::var_os("USERPROFILE")
.filter(|profile| !profile.is_empty())
.or_else(|| std::env::var_os("HOME"))
.map(PathBuf::from);
}
std::env::var_os("HOME")
.or_else(|| std::env::var_os("USERPROFILE"))
.map(PathBuf::from)
Expand Down Expand Up @@ -1561,4 +1578,106 @@ mod tests {
failures.join("\n")
);
}

/// The state directory and the discovery file in it both name the `hyperd`
/// endpoint, so both must be restricted to the owning user — including when
/// an earlier release already created them under the process umask.
///
/// Asserts the mode actually on disk rather than that a `chmod` was
/// attempted, since only the former is what another local account sees.
#[cfg(unix)]
fn run_state_permissions_scenario() {
use std::os::unix::fs::PermissionsExt as _;

assert!(
std::env::var_os("HYPERDB_STATE_DIR").is_some(),
"child scenario requires an isolated state directory"
);

fn mode_of(path: &Path) -> u32 {
std::fs::metadata(path)
.unwrap_or_else(|error| panic!("{} should exist: {error}", path.display()))
.permissions()
.mode()
& 0o777
}

let dir = state_dir().unwrap();
let path = discovery_file_path().unwrap();
let mut failures = Vec::new();

// A state directory and discovery file created from nothing.
write_discovery_file(&legacy_info()).unwrap();
let fresh_dir_mode = mode_of(&dir);
if fresh_dir_mode != 0o700 {
failures.push(format!(
"a newly created state directory was left at {fresh_dir_mode:04o} instead of \
0700, so another local account can traverse it"
));
}
let fresh_file_mode = mode_of(&path);
if fresh_file_mode != 0o600 {
failures.push(format!(
"a newly written discovery file was left at {fresh_file_mode:04o} instead of \
0600, so it is readable by other local accounts"
));
}

// Permissions left loose by an earlier release must be corrected on the
// next write rather than accepted as they are.
std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o755)).unwrap();
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).unwrap();
assert_eq!(
(mode_of(&dir), mode_of(&path)),
(0o755, 0o644),
"fixture must start world-readable or it does not exercise the correction"
);

write_discovery_file(&legacy_info()).unwrap();
let corrected_dir_mode = mode_of(&dir);
if corrected_dir_mode != 0o700 {
failures.push(format!(
"a pre-existing world-readable state directory stayed at {corrected_dir_mode:04o} \
instead of being tightened to 0700"
));
}
let corrected_file_mode = mode_of(&path);
if corrected_file_mode != 0o600 {
failures.push(format!(
"a pre-existing world-readable discovery file stayed at {corrected_file_mode:04o} \
instead of being tightened to 0600"
));
}

// The record must still be readable and intact afterwards: tightening
// permissions is worthless if it breaks the daemon's own discovery.
match serde_json::from_slice::<DaemonInfo>(&std::fs::read(&path).unwrap()) {
Ok(parsed) if parsed == legacy_info() => {}
Ok(_) => failures.push("the restricted record did not round-trip".to_string()),
Err(error) => {
failures.push(format!("the restricted record was unreadable: {error}"));
}
}

assert!(
failures.is_empty(),
"daemon state permission failures:\n{}",
failures.join("\n")
);
}

#[cfg(unix)]
#[test]
fn state_directory_and_discovery_file_are_owner_only() {
const CHILD_SENTINEL_ENV: &str = "HYPERDB_MCP_STATE_PERMISSIONS_CHILD";
const TEST_NAME: &str =
"daemon::discovery::tests::state_directory_and_discovery_file_are_owner_only";

if let Some(marker) = std::env::var_os(CHILD_SENTINEL_ENV) {
std::fs::write(std::path::PathBuf::from(marker), b"started").unwrap();
run_state_permissions_scenario();
return;
}
run_discovery_compatibility_child(TEST_NAME, CHILD_SENTINEL_ENV);
}
}
1 change: 1 addition & 0 deletions hyperdb-mcp/src/daemon/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub mod discovery;
pub mod health;
pub mod run;
pub mod spawn;
pub mod state_perms;

/// Default base TCP port for the daemon health listener. When no env var is set,
/// the daemon scans `[base, base + DAEMON_PORT_SCAN_SPAN)` to find a free port.
Expand Down
12 changes: 10 additions & 2 deletions hyperdb-mcp/src/daemon/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,16 @@ pub fn try_record_restart_attempt(history: &mut Vec<Instant>, now: Instant) -> R

/// Build the Parameters used for every hyperd spawn (initial start and restarts).
fn build_params() -> std::io::Result<Parameters> {
let log_dir = discovery::state_dir()?.join("logs");
std::fs::create_dir_all(&log_dir)?;
// The state directory holds `daemon.json`; `logs/` holds `hyperd`'s own
// diagnostic logs, which name the endpoint just as `daemon.json` does.
// `hyperd` is a separate process writing under its own umask, so
// restricting the directory is what covers those files. Both levels are
// restricted here so the daemon's own startup establishes the invariant
// instead of it depending on the later discovery-file write.
let state_dir = discovery::state_dir()?;
super::state_perms::ensure_owner_only_dir(&state_dir)?;
let log_dir = state_dir.join("logs");
super::state_perms::ensure_owner_only_dir(&log_dir)?;

let mut params = Parameters::new();
params.set("log_file_max_count", "2");
Expand Down
Loading