Skip to content

fix(daemon): the data directory is created owner-only - #892

Merged
V3RON merged 1 commit into
security/exec-envfrom
security/data-dir-mode
Oct 26, 2026
Merged

V3RON merged 1 commit into
security/exec-envfrom
security/data-dir-mode

Conversation

@V3RON

@V3RON V3RON commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The security model says file modes are the real boundary, but the data directory (~/.simlock, ~/.simlock/gateway) was created with the umask default, usually 0755. Now whatever creates it first makes it 0700. An existing directory keeps its mode. When the group or others can reach it, the daemon logs one warn at start: data directory <path> is accessible to other users (mode <octal>); run chmod 700 <path>.

Changes

  • Filesystem.mkdirp(path, { mode }): the mode applies only to a directory the call creates; the Node adapter runs chmod after mkdir so the umask cannot drop bits. DATA_DIRECTORY_MODE = 0o700 in src/ports/paths.ts.
  • Daemon: prepareDataDirectory runs first in composeDaemon; DaemonEndpointHost.start creates the socket's directory 0700.
  • CLI: config set, the auto-launch and service install create the directory 0700.
  • Docs: worker/running-the-daemon.mdx, manuals for src/ports, src/daemon, src/cli, and flows/startup.md.

Tests

  • Filesystem contract over both implementations, the endpoint host, startDaemon (missing, 0700, 0750, 0755), the launcher, service install, and e2e/daemon-folders.test.ts.

Assumptions

  • Assumption: should the CLI auto-launch and service install also create the directory 0700? — yes; on a fresh machine they create it first, and the daemon would otherwise warn about its own launcher's directory.
  • Assumption: where does the daemon create and check the directory? — at the top of composeDaemon, before the log sink; later mkdirp calls stay plain because the directory exists by then.
  • Assumption: should NodeDaemonLauncher import DATA_DIRECTORY_MODE? — no, it uses the literal 0o700, because a test loads it alone in a child process that cannot resolve local imports.
  • Assumption: what counts as "accessible to other users"? — any group or other bit (mode & 0o077); the warning repeats on every start until fixed.

Review

Spec review: skipped. Code review: 0 blocking, 0 fixed, 0 notes. Claims review: skipped.
Rounds: 1. Tokens: n/a.

One combined review by the maintainer's agent on 2026-10-22 (comment below); no blocking findings. Its one fix was withdrawn in a follow-up comment.

Rebased onto main e1f2f43 on 2026-10-26 as one commit. No conflicts.

Written by an agent.

@V3RON

V3RON commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Review (maintainer's agent, round 1)

One fix, otherwise sound.

Fix

  • src/cli/environment.ts writeConfigFile: const { DATA_DIRECTORY_MODE } = await import("../ports/index.js") is a dynamic import of a module the file already imports statically at line 18. Add DATA_DIRECTORY_MODE to that static import and drop the await import.

Checked and fine

  • NodeFilesystem.mkdirp(path, { mode }) applies the mode only to the last directory and only when this call created it (EEXIST returns before chmod), so a user's existing mode is never changed; the chmod after mkdir follows device-root.ts against the umask. The memory fake mirrors this and its stat now reports the real directory mode instead of a constant, which the daemon tests needed.
  • prepareDataDirectory runs at the top of composeDaemon, before the log sink creates the directory, and the warning is logged once the logger exists; the four-row table test on a real temp dir covers missing, 0700, 0750 and 0755.
  • Covering the auto-launcher and the service install too was right: without them a first simlock status would create ~/.simlock 0755 and the daemon would then warn about its own launcher's directory. The literal 0o700 in daemon-launcher.ts is justified by its isolated child-process test and says so.
  • Website paragraph says what happens, what the warning means and what to run.

Note, no change needed: on a gateway-only machine <root> itself stays at the umask default and only <root>/gateway is 0700, as decided ("never their parents").

@V3RON

V3RON commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Review round 1, correction

The one fix above is withdrawn. Line 18 of src/cli/environment.ts is an import type block, and the file loads ../ports/index.js lazily on purpose (its header says nothing heavy is imported statically so the CLI starts fast); every other runtime value from ports in that file goes through await import. The new writeConfigFile line follows the same pattern and should stay as it is.

No blocking findings remain.

@V3RON
V3RON changed the base branch from main to security/exec-env October 23, 2026 06:19
@V3RON
V3RON added this pull request to stack #897 October 23, 2026 06:19
@V3RON
V3RON marked this pull request as ready for review October 26, 2026 09:46
@V3RON
V3RON force-pushed the security/data-dir-mode branch from 53935ba to 4671805 Compare October 26, 2026 10:29
@V3RON
V3RON force-pushed the security/data-dir-mode branch from 4671805 to 68d941d Compare October 26, 2026 14:25
@V3RON
V3RON merged commit 6732003 into main Oct 26, 2026
24 of 25 checks passed
@V3RON
V3RON deleted the security/data-dir-mode branch October 26, 2026 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant