Skip to content

Client-mode log directory is created under the umask, unlike the daemon's state directory #297

Description

@StefanSteiner

Summary

The daemon's state directory and logs/ are created with an explicit owner-only mode
(daemon::state_perms, added in #295). The client/local-mode log directory is not: both
call sites use a plain std::fs::create_dir_all, so it takes the process umask — commonly
0755.

That directory holds the same class of file. In local mode Engine::new passes it to hyperd
as the engine's own log_dir, so hyperd writes its diagnostic logs there, and those records
name the endpoint just as the daemon's logs/ do. Restricting the daemon's state directory
while leaving the client's log directory at the umask is an inconsistency rather than a
deliberate difference.

Where

  • engine::resolve_log_dir (hyperdb-mcp/src/engine.rs) returns either the persistent file's
    parent, or std::env::temp_dir().join(format!("hyperdb-mcp-{pid}")) when the session is
    ephemeral.
  • Both call sites create it with a plain create_dir_all:
    • hyperdb-mcp/src/main.rs (client-mode tracing setup, writes hyperdb-mcp.log)
    • hyperdb-mcp/src/engine.rs (Engine::new)
  • Engine::new then sets params.set("log_dir", …) for the local HyperProcess, so hyperd
    rotates its own logs into the same directory.
  • The adjacent ephemeral data directory (hyperdb-mcp-<pid>-<seq>, holding the session's
    .hyper files) is created the same way and is worth considering in the same pass.

Why it varies by platform

The ephemeral case is the one that matters, and how much depends on the platform:

  • Linuxtemp_dir() is /tmp, which is shared and world-traversable, and the directory
    name is just the pid. This is the case worth fixing.
  • macOStemp_dir() is a per-user /var/folders/… directory that is already 0700, so
    the parent covers it.
  • Windows — the per-user temp directory sits inside the user profile and inherits its ACL.

Suggested fix

Reuse daemon::state_perms::ensure_owner_only_dir at both call sites instead of
create_dir_all. It already creates at 0700, tightens a pre-existing directory, sweeps the
regular files inside it, and warns rather than failing when the filesystem has no modes to set
— the same policy the daemon paths use, which is what makes this a consistency fix rather than
a new one. Consider the ephemeral data directory too.

Notes

  • Ordinary file-permission hygiene, not an urgent defect: on macOS and Windows the enclosing
    directory already restricts access, and on Linux it affects a local developer tool's own
    diagnostic output.
  • Deliberately out of scope for fix(mcp): restrict daemon state files to the owning user #295, which is about the daemon's state directory.
  • Whether the client's own hyperdb-mcp.log records the endpoint was not established —
    no tracing call in engine.rs, server.rs or main.rs emits it as a field, though the
    endpoint does appear in the text of a connect-failure message. The hyperd logs in the same
    directory are the clear case, and they are enough to motivate the change.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions