From 68d941d198a224c0c2ddaaf9c180e285c5144e4f Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Mon, 26 Oct 2026 10:59:54 +0100 Subject: [PATCH] fix(daemon): the data directory is created owner-only (#892) --- docs/internal/flows/startup.md | 4 +- e2e/daemon-folders.test.ts | 5 +- src/cli/AGENTS.md | 4 +- src/cli/environment.ts | 3 +- src/daemon/AGENTS.md | 9 ++- src/daemon/connection-host.test.ts | 28 ++++++++ src/daemon/connection-host.ts | 20 +++++- src/daemon/main.test.ts | 72 ++++++++++++++++++- src/daemon/main.ts | 3 + src/ports/AGENTS.md | 6 +- src/ports/daemon-launcher.test.ts | 11 +-- src/ports/daemon-launcher.ts | 22 +++++- src/ports/filesystem.test.ts | 23 ++++++ src/ports/filesystem.ts | 66 +++++++++++++++-- src/ports/index.ts | 1 + src/ports/paths.ts | 8 +++ src/ports/service-manager.test.ts | 15 +++- src/ports/service-manager.ts | 5 +- .../docs/docs/worker/running-the-daemon.mdx | 2 + 19 files changed, 274 insertions(+), 33 deletions(-) diff --git a/docs/internal/flows/startup.md b/docs/internal/flows/startup.md index 71b40ea41..a18c5833e 100644 --- a/docs/internal/flows/startup.md +++ b/docs/internal/flows/startup.md @@ -5,7 +5,9 @@ Entry: `src/daemon/main.ts` `startDaemon` → `DaemonServer.start` (`src/daemon/ ## Both modes, before the branch -1. `loadConfig` reads `config.json`; its warnings are logged once the logger exists. +1. `prepareDataDirectory` creates the data directory `0700` when missing; an existing one keeps + its mode, and one group or others can reach is a `warn` line once the logger exists. + `loadConfig` reads `config.json`; its warnings are logged once the logger exists. 2. `Logger` → `daemon.log`. At `log.level: debug` the `ProcessRunner` is wrapped. 3. Host info read once (`sw_vers` on macOS); bounded, never throws. 4. `EventBus` → the event history is created (`createEventHistory`). It touches no file yet. diff --git a/e2e/daemon-folders.test.ts b/e2e/daemon-folders.test.ts index 86b00f187..3486d9e31 100644 --- a/e2e/daemon-folders.test.ts +++ b/e2e/daemon-folders.test.ts @@ -1,6 +1,6 @@ import { spawn } from "node:child_process"; import { existsSync } from "node:fs"; -import { readFile, writeFile } from "node:fs/promises"; +import { readFile, stat, writeFile } from "node:fs/promises"; import { createServer } from "node:net"; import { join } from "node:path"; import { DatabaseSync } from "node:sqlite"; @@ -79,12 +79,13 @@ describe("daemon folders (ADR 0034)", () => { expect(result.stdout).toContain("Usage: simlock"); }); - it("--gateway config set makes a gateway present, after which --gateway status starts it", async () => { + it("--gateway config set makes a gateway present in an owner-only folder, after which --gateway status starts it", async () => { const env = await withDaemon({ mode: "auto" }); try { await makeGatewayPresent(env); expect(existsSync(join(gatewayFolder(env), "config.json"))).toBe(true); + expect((await stat(gatewayFolder(env))).mode & 0o777).toBe(0o700); const status = await env.cli(["--gateway", "status", "--json"]); diff --git a/src/cli/AGENTS.md b/src/cli/AGENTS.md index 63059dc43..2951b9588 100644 --- a/src/cli/AGENTS.md +++ b/src/cli/AGENTS.md @@ -96,8 +96,8 @@ Surface: [cli.mdx](../../website/src/content/docs/docs/reference/cli.mdx) — ev 6a. **`simlock stats`** — `usageWindow` (contract) turns `--since`, `--from`, `--to` into a window; each refusal is a usage error (exit 2) worded here. Asks `usage.get` and prints `formatUsage` (`stats.ts`) or the answer as `--json`; computes no figure. `--csv` asks with `includeLedger` and writes `ledgerCsv` (`csv.ts`, RFC 4180, CRLF, a leading `'` before `=`, `+`, `-`, `@`); `--csv` with `--json` is a usage error; a `ledgerTruncated` answer adds a stderr notice. 7. **`simlock daemon logs --follow`** — `followLog` polls every 250 ms from a byte offset. A new identity on `daemon.log.1` is a rotation: print its rest, restart at offset 0. -8. **`simlock config set`** — validates the merged file with `loadConfig`, writes - `config.json` atomically, mode `0600`. +8. **`simlock config set`** — validates the merged file with `loadConfig`, creates the + data directory `0700` when missing, writes `config.json` atomically, mode `0600`. 9. **`simlock mcp`** — imports `src/mcp/main.ts` and runs it, passing the global flag as `{ daemonFlag }`. 10. **`simlock instructions [--json]`** — prints the static rules text from `src/instructions`, or one JSON object `{ instructions }`; never connects to a daemon. diff --git a/src/cli/environment.ts b/src/cli/environment.ts index 70934041d..aaa7c27d7 100644 --- a/src/cli/environment.ts +++ b/src/cli/environment.ts @@ -392,7 +392,8 @@ export function buildCliEnvironment( import("./passthrough.js").then(({ spawnPassthrough }) => spawnPassthrough(command))), readStdin: ports.readStdin ?? readPipedStdin, writeConfigFile: async (contents) => { - await filesystem.mkdirp(dataDirectory); + const { DATA_DIRECTORY_MODE } = await import("../ports/index.js"); + await filesystem.mkdirp(dataDirectory, { mode: DATA_DIRECTORY_MODE }); // why: Owner-only on every write: the file can hold `gateway.token`, and the atomic rename // replaces the old file, so a mode the user set by hand would not survive otherwise. await filesystem.writeFileAtomic(configPath, `${JSON.stringify(contents, null, 2)}\n`, { diff --git a/src/daemon/AGENTS.md b/src/daemon/AGENTS.md index 554348737..70eb59cfe 100644 --- a/src/daemon/AGENTS.md +++ b/src/daemon/AGENTS.md @@ -30,7 +30,7 @@ Flows: [startup](../../docs/internal/flows/startup.md), [device.exec](../../docs ## Composition (`main.ts`) -Both modes: config (`config.json`) → `Logger` → host info → `EventBus` → event history (`createEventHistory`, not yet started) → `openStateDatabase` (the role's `state.db`; a gateway's holds the `tokens` table only; closed last in `dispose`, or by `startDaemon` when the start fails after the open). +Both modes: the data directory (`prepareDataDirectory`: created `0700` when missing; an existing one keeps its mode, and one group or others can reach gets one `warn` line once the logger exists) → config (`config.json`) → `Logger` → host info → `EventBus` → event history (`createEventHistory`, not yet started) → `openStateDatabase` (the role's `state.db`; a gateway's holds the `tokens` table only; closed last in `dispose`, or by `startDaemon` when the start fails after the open). The role (`StartDaemonOptions.role`; the entry reads it from `--gateway` through `parseDaemonArgv`) then picks one branch; a gateway builds none of the worker's parts. The data directory is `daemonDataDirectory(resolveSimlockHome(), role)`: `` for a worker, `/gateway` for a gateway. The daemon never reads the other role's folder. - **Worker** — `openStateDatabase` (`state.db`, before the token checks) → `Registry` → instance id (`instance.json`) → @@ -55,7 +55,7 @@ Its stdout lines move the state; a failed line, a non-zero exit or a failed laun ## Connection (`server.ts`) -1. **Claim** — `DaemonEndpointHost.start`: a live peer → `DaemonAlreadyRunningError`; a dead socket file is removed and claimed. Default `/daemon.sock`. +1. **Claim** — `DaemonEndpointHost.start`: creates the socket's directory `0700` when missing (an existing one keeps its mode); a live peer → `DaemonAlreadyRunningError`; a dead socket file is removed and claimed. Default `/daemon.sock`. 2. **Hello** — first frame must be `hello`, else `HANDSHAKE_REQUIRED` and close. Role: the uplink connection is `admin` by acceptance; otherwise the resolver. Principal: `hello.principal`, else `SIMLOCK_AGENT_ID`, else the daemon's pid. Both are fixed for the connection's life. 3. **Version** — ranges overlap → highest common version. `PROTOCOL_VERSION_RANGE` @@ -107,7 +107,10 @@ Admin secret: fresh each start, hash kept in memory. Written to `admin.token` (0 Security model: cooperative identity, not a hostile-process boundary. -- Every socket peer is the same OS user; file modes are the real boundary. +- Every socket peer is the same OS user; file modes are the real boundary: the data + directory is created `0700` (`DATA_DIRECTORY_MODE`) by whoever makes it first (the + daemon, `NodeDaemonLauncher`, `config set`, service install), and its files are `0600`. + An existing directory is never chmod'ed; the daemon warns when group or others can reach it. - A `hello.principal` is a claim. Ownership checks stop accidents, not attacks. - Admin comes only from a credential at `hello`, never from the socket itself. - A worker that joins a fleet grants its gateway `admin` over itself. diff --git a/src/daemon/connection-host.test.ts b/src/daemon/connection-host.test.ts index 3c0996252..e1d0a9eb8 100644 --- a/src/daemon/connection-host.test.ts +++ b/src/daemon/connection-host.test.ts @@ -1,3 +1,7 @@ +import { mkdtemp, rm, stat } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + import { describe, expect, it } from "vitest"; import { @@ -7,6 +11,7 @@ import { MemoryFilesystem, MemoryIpcTransport, MemoryLogSink, + NodeFilesystem, type IpcConnection, type IpcConnector, type IpcListenerFactory, @@ -30,6 +35,29 @@ describe("DaemonEndpointHost", () => { await expect(ipc.connect(endpoint)).rejects.toMatchObject({ code: "endpoint-not-found" }); }); + it("creates the directory its endpoint lives in owner-only", async () => { + // why: under the umask every real host starts from, a directory made without a mode comes + // out 0755, so only a mode the host asks for ends at 0700. + const previousUmask = process.umask(0o022); + const base = await mkdtemp(join(tmpdir(), "simlock-endpoint-")); + const ipc = new MemoryIpcTransport(); + const host = new DaemonEndpointHost({ + connector: ipc, + endpoint: join(base, "home", "daemon.sock"), + filesystem: new NodeFilesystem(), + listenerFactory: ipc, + }); + try { + await host.start(() => undefined); + + expect((await stat(join(base, "home"))).mode & 0o777).toBe(0o700); + } finally { + process.umask(previousUmask); + await host.stop(); + await rm(base, { force: true, recursive: true }); + } + }); + it("rejects a live endpoint before binding", async () => { const filesystem = new MemoryFilesystem(); await filesystem.mkdirp("/simlock"); diff --git a/src/daemon/connection-host.ts b/src/daemon/connection-host.ts index abac5e1af..3cb515ddb 100644 --- a/src/daemon/connection-host.ts +++ b/src/daemon/connection-host.ts @@ -7,7 +7,7 @@ import type { IpcListenerFactory, Logger, } from "../ports/index.js"; -import { IpcError, NoopLogger } from "../ports/index.js"; +import { DATA_DIRECTORY_MODE, IpcError, NoopLogger } from "../ports/index.js"; export interface ConnectionHost { readonly endpoint: string; @@ -40,6 +40,22 @@ export async function refuseWhenDaemonAnswers(options: { throw new DaemonAlreadyRunningError(options.endpoint); } +/** + * why: the daemon makes its data directory owner-only before anything writes into it (the log, + * `state.db`, the socket). One that already exists is never chmod'ed, since the user chose its + * mode: this only returns the warning the daemon logs when the group or others can reach it. + */ +export async function prepareDataDirectory( + filesystem: Filesystem, + path: string, +): Promise { + await filesystem.mkdirp(path, { mode: DATA_DIRECTORY_MODE }); + const mode = ((await filesystem.stat(path)).mode ?? DATA_DIRECTORY_MODE) & 0o777; + if ((mode & 0o077) === 0) return undefined; + const octal = mode.toString(8).padStart(3, "0"); + return `data directory ${path} is accessible to other users (mode ${octal}); run chmod 700 ${path}`; +} + export interface DaemonEndpointHostOptions { readonly connector: IpcConnector; readonly endpoint: string; @@ -65,7 +81,7 @@ export class DaemonEndpointHost implements ConnectionHost { async start(accept: (connection: IpcConnection) => void): Promise { if (this.#listener !== undefined || this.#stopped) throw new Error("Daemon endpoint host has already been started"); - await this.options.filesystem.mkdirp(dirname(this.endpoint)); + await this.options.filesystem.mkdirp(dirname(this.endpoint), { mode: DATA_DIRECTORY_MODE }); if (await this.options.filesystem.exists(this.endpoint)) { try { const live = await this.options.connector.connect(this.endpoint); diff --git a/src/daemon/main.test.ts b/src/daemon/main.test.ts index d447e03da..f6c560251 100644 --- a/src/daemon/main.test.ts +++ b/src/daemon/main.test.ts @@ -1,5 +1,5 @@ import { existsSync } from "node:fs"; -import { mkdir, mkdtemp, readdir, readFile, rm, stat, writeFile } from "node:fs/promises"; +import { chmod, mkdir, mkdtemp, readdir, readFile, rm, stat, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { createServer as createHttpServer } from "node:http"; @@ -53,6 +53,7 @@ import { JsonLinesLogger, MemoryFilesystem, MemoryLogSink, + NodeFilesystem, NodeSqliteOpener, NoopLogger, ScriptedProcessRunner, @@ -300,6 +301,75 @@ const hasEvent = (events: readonly EventEnvelope[]): boolean => events.some((event) => event.event === name); +describe("the data directory", () => { + it("creates a missing data directory owner-only, and for one other users can reach logs one warning at start and leaves its mode", async () => { + // why: under the umask every real host starts from, a directory made without a mode comes + // out 0755, so only the daemon asking for 0700 ends there. + const previousUmask = process.umask(0o022); + try { + const base = await mkdtemp(join(tmpdir(), "simlock-dd-")); + temporaryDirectories.push(base); + const cases = [ + { name: "new", existing: undefined, mode: 0o700, warned: undefined }, + { name: "700", existing: 0o700, mode: 0o700, warned: undefined }, + { name: "750", existing: 0o750, mode: 0o750, warned: "750" }, + { name: "755", existing: 0o755, mode: 0o755, warned: "755" }, + ]; + + for (const testCase of cases) { + const dataDirectory = join(base, testCase.name); + if (testCase.existing !== undefined) { + await mkdir(dataDirectory); + await chmod(dataDirectory, testCase.existing); + } + + const { sink } = await start({ dataDirectory, filesystem: new NodeFilesystem() }); + + expect( + { + mode: (await stat(dataDirectory)).mode & 0o777, + warnings: sink.records + .filter((record) => record.message.startsWith("data directory")) + .map((record) => [record.level, record.message]), + }, + testCase.name, + ).toEqual({ + mode: testCase.mode, + warnings: + testCase.warned === undefined + ? [] + : [ + [ + "warn", + `data directory ${dataDirectory} is accessible to other users (mode ${testCase.warned}); run chmod 700 ${dataDirectory}`, + ], + ], + }); + } + } finally { + process.umask(previousUmask); + } + }); +}); + +describe("the data directory", () => { + it("makes the parent of a gateway's data directory owner-only too when it creates it", async () => { + // why: a gateway started first creates ``, which is the worker's data directory later. + const previousUmask = process.umask(0o022); + try { + const root = join(await mkdtemp(join(tmpdir(), "simlock-dd-")), "home"); + temporaryDirectories.push(join(root, "..")); + + await start({ dataDirectory: join(root, "gateway"), filesystem: new NodeFilesystem() }); + + expect((await stat(root)).mode & 0o777).toBe(0o700); + expect((await stat(join(root, "gateway"))).mode & 0o777).toBe(0o700); + } finally { + process.umask(previousUmask); + } + }); +}); + describe("the version a daemon reports", () => { it("is the version in package.json when the caller names none", async () => { const directory = await mkdtemp(join(tmpdir(), "simlock-main-")); diff --git a/src/daemon/main.ts b/src/daemon/main.ts index 1e5e1e3d1..36bcf68ad 100644 --- a/src/daemon/main.ts +++ b/src/daemon/main.ts @@ -143,6 +143,7 @@ import { startVersionCheck, type VersionCheck } from "./version-check.js"; import { DaemonAlreadyRunningError, DaemonEndpointHost, + prepareDataDirectory, refuseWhenDaemonAnswers, } from "./connection-host.js"; import { describeLeaseRequestFailure } from "./error-code.js"; @@ -279,6 +280,7 @@ async function composeDaemon( const tcpProbe = options.tcpProbe ?? new NodeTcpProbe(); const configPath = options.configPath ?? join(dataDirectory, "config.json"); const socketPath = options.socketPath ?? resolveDaemonSocketPath(dataDirectory); + const dataDirectoryWarning = await prepareDataDirectory(filesystem, dataDirectory); const configWarnings: string[] = []; const moduleDefinitions = options.optionalModules ?? optionalModules; const { config, defaulted } = await loadConfigWithDefaults({ @@ -306,6 +308,7 @@ async function composeDaemon( }); // why: the logger needs the config's log level, so what the config load found to warn about is told once the logger exists. for (const message of configWarnings) logger.warn(message); + if (dataDirectoryWarning !== undefined) logger.warn(dataDirectoryWarning); const processRunner = processRunnerFor(config.log.level, baseProcessRunner, logger, clock); // ADR 0008 §6: the machine is read once, here. Bounded and never throwing, so a slow or // missing `sw_vers` costs a fallback value, not the daemon. diff --git a/src/ports/AGENTS.md b/src/ports/AGENTS.md index 0ef87a20e..273372898 100644 --- a/src/ports/AGENTS.md +++ b/src/ports/AGENTS.md @@ -10,12 +10,12 @@ Uses: nothing from other components. Imported by: every component that touches t | Port | File | Node adapter | Fake | What it covers | | ------------------------------------------ | --------------------------- | --------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | `Clock` | `clock.ts` | `SystemClock` | `FakeClock` | `now`, `setTimer`, `cancel` | -| `Filesystem` | `filesystem.ts` | `NodeFilesystem` | `MemoryFilesystem` | Read, atomic write, copy, stat, delete, disk free | +| `Filesystem` | `filesystem.ts` | `NodeFilesystem` | `MemoryFilesystem` | Read, atomic write, copy, stat, delete, disk free; `mkdirp(path, { mode })` gives every directory it creates, parents included, and none that exists, exactly that mode (chmod past the umask) | | `ProcessRunner` | `process-runner.ts` | `NodeProcessRunner` | `ScriptedProcessRunner` | Run or stream a command, capture output | | (wrapper) | `logging-process-runner.ts` | `LoggingProcessRunner` | — | One `process` log line per command, at `debug` only | | `ProcessSupervisor` | `process-supervisor.ts` | `NodeProcessSupervisor` | `FakeProcessSupervisor` | Is a pid alive; signal it | | `ParentWatch` | `parent-watch.ts` | `NodeParentWatch` | `FakeParentWatch` | Call once when a pid exits (polls) | -| `DaemonLauncher` | `daemon-launcher.ts` | `NodeDaemonLauncher` | `FakeDaemonLauncher` | Detached daemon, its log, `DAEMON_STARTUP_TIMEOUT_MS`; `launch()` answers a `LaunchedDaemon` (`exited`, `readOutput` of the log since the launch); `startErrorFrom` | +| `DaemonLauncher` | `daemon-launcher.ts` | `NodeDaemonLauncher` | `FakeDaemonLauncher` | Detached daemon, its log (and the data directory, `0700`, when missing), `DAEMON_STARTUP_TIMEOUT_MS`; `launch()` answers a `LaunchedDaemon` (`exited`, `readOutput` of the log since the launch); `startErrorFrom` | | `NpmCli` | `npm-cli.ts` | `NodeNpmCli` | `FakeNpmCli` | `view`, `install`, `globalRoot`: runs `process.execPath` with the real path of the `npm` beside it, in the daemon's data directory; 30 s for `view` and `globalRoot`, 10 min for `install`, the child killed at the limit; an error is the last 2000 characters of stderr | | `UpdaterLauncher` | `updater-launcher.ts` | `NodeUpdaterLauncher` | `FakeUpdaterLauncher` | `launch` spawns the built entry of `src/updater/main.ts` detached with the Node that runs it: stdout a pipe (`lines`), stderr appended to the log, `exited` the exit code | | `ServiceManager` | `service-manager.ts` | `LaunchdServiceManager`, `SystemdServiceManager` (`productionServiceManager(platform)`: `darwin`, `linux`; `undefined` elsewhere) | `FakeServiceManager` | A per-user service for one role: `state` (`not-installed`, `installed`, `not-loaded`, `broken`), `install`, `uninstall`, `start` (kickstart); every `launchctl`/`plutil`/`systemctl --user` call has a 10 s limit; systemd writes `$XDG_CONFIG_HOME/systemd/user/.service` (absolute only, else `~/.config`); `serviceDefinition` builds the name, path, command and saved variables | @@ -34,7 +34,7 @@ Uses: nothing from other components. Imported by: every component that touches t | `WebhookPoster` | `webhook-poster.ts` | `FetchWebhookPoster` | `FakeWebhookPoster` (`testing.ts`) | POST to an operator-named URL; a redirect is not followed | | `IdGenerator` | `id-generator.ts` | `CryptoIdGenerator` | none; tests inline one | Random ids | | `TokenSecrets` | `token-secrets.ts` | `CryptoTokenSecrets` | none; tests inline one | New secret, its hash | -| Paths | `paths.ts` | `resolveSimlockHome`, `daemonDataDirectory`, `resolveDaemonHome`, `resolveDaemonSocketPath` | — | `SIMLOCK_HOME` (test root; default `~/.simlock`), a gateway in `/gateway`, socket path length; `resolveDaemonHome` picks the folder from the flag (`gateway`, `local`) or, with none, the gateway only when `/gateway/{instance,config}.json` exists and `/instance.json` does not | +| Paths | `paths.ts` | `resolveSimlockHome`, `daemonDataDirectory`, `DATA_DIRECTORY_MODE` (`0700`), `resolveDaemonHome`, `resolveDaemonSocketPath` | — | `SIMLOCK_HOME` (test root; default `~/.simlock`), a gateway in `/gateway`, socket path length; `resolveDaemonHome` picks the folder from the flag (`gateway`, `local`) or, with none, the gateway only when `/gateway/{instance,config}.json` exists and `/instance.json` does not | `productionHostLoadProbe` picks the Darwin or Linux probe. `NodeFileLogSink` rotates to `.1` and keeps that one generation; the files it opens, and the log `NodeDaemonLauncher` diff --git a/src/ports/daemon-launcher.test.ts b/src/ports/daemon-launcher.test.ts index 179024a3a..ef08c55dc 100644 --- a/src/ports/daemon-launcher.test.ts +++ b/src/ports/daemon-launcher.test.ts @@ -49,14 +49,14 @@ describe("NodeDaemonLauncher", () => { } }); - it("creates the daemon log it hands the child owner-only", async () => { + it("creates the daemon log it hands the child, and the data directory it lives in, owner-only", async () => { // why: #685: when the CLI or MCP server auto-launches the daemon, this `open(path, "a")` is - // what first creates `daemon.log` -- the daemon's own sink only appends afterwards. A - // restrictive umask in the test environment would mask what the code asks for, so pin - // the one every real host starts from. + // what first creates `daemon.log` -- the daemon's own sink only appends afterwards -- and on + // a fresh machine the launch creates the data directory too. A restrictive umask in the test + // environment would mask what the code asks for, so pin the one every real host starts from. const previousUmask = process.umask(0o022); const directory = await mkdtemp(join(tmpdir(), "simlock-launcher-")); - const logPath = join(directory, "daemon.log"); + const logPath = join(directory, "home", "daemon.log"); const launcher = new NodeDaemonLauncher({ args: ["-e", "console.log('out')"], command: process.execPath, @@ -68,6 +68,7 @@ describe("NodeDaemonLauncher", () => { await expect.poll(async () => (await readFile(logPath, "utf8")).trim()).toBe("out"); expect((await stat(logPath)).mode & 0o777).toBe(0o600); + expect((await stat(join(directory, "home"))).mode & 0o777).toBe(0o700); } finally { process.umask(previousUmask); await rm(directory, { force: true, recursive: true }); diff --git a/src/ports/daemon-launcher.ts b/src/ports/daemon-launcher.ts index e4a2c7c67..69afa5ed5 100644 --- a/src/ports/daemon-launcher.ts +++ b/src/ports/daemon-launcher.ts @@ -1,5 +1,5 @@ import { spawn } from "node:child_process"; -import { mkdir, open, readFile, stat } from "node:fs/promises"; +import { chmod, mkdir, open, readFile, stat } from "node:fs/promises"; import { dirname } from "node:path"; /** Starts the detached daemon owned by the operating system rather than the CLI. */ @@ -85,11 +85,29 @@ export interface NodeDaemonLauncherOptions { readonly simlockHome: string; } +/** + * why: the log lives in the data directory, and on a fresh machine this launch is what first + * creates it, so it is made owner-only here (`DATA_DIRECTORY_MODE`, which a test that loads this + * file alone in a child process cannot import), its parents left to the default and an existing + * one left as it is. `chmod` because the umask can only take bits from `mkdir`'s mode. + */ +async function createDataDirectory(directory: string): Promise { + await mkdir(dirname(directory), { recursive: true }); + try { + await mkdir(directory, { mode: 0o700 }); + } catch (error: unknown) { + if (typeof error === "object" && error !== null && "code" in error && error.code === "EEXIST") + return; + throw error; + } + await chmod(directory, 0o700); +} + export class NodeDaemonLauncher implements DaemonLauncher { constructor(private readonly options: NodeDaemonLauncherOptions) {} async launch(): Promise { - await mkdir(dirname(this.options.logPath), { recursive: true }); + await createDataDirectory(dirname(this.options.logPath)); // why: 0600 at creation, like the other files that hold secrets or state under the home: // this `open` is what first creates `daemon.log` for an auto-launched daemon -- the daemon's // own sink only appends to the file this one made. diff --git a/src/ports/filesystem.test.ts b/src/ports/filesystem.test.ts index 6c07ebbcd..603def6ee 100644 --- a/src/ports/filesystem.test.ts +++ b/src/ports/filesystem.test.ts @@ -98,6 +98,29 @@ describe.each(implementations)("Filesystem contract: $name", ({ create, link }) await expect(filesystem.stat(directory)).resolves.toMatchObject({ kind: "directory" }); }); + it("gives every directory it creates, parents included, the mode it asks for, past the umask, and leaves an existing directory alone", async () => { + // why: under umask 0o077 Node's own default for a new directory is 0o700, so a mode of 0o750 + // the umask would strip to 0o700 shows the adapter set the bits after mkdir, on the parent too. + const previousUmask = process.umask(0o077); + try { + const filesystem = create(); + const directory = `${temporaryDirectory}/home/gateway`; + const existing = `${temporaryDirectory}/existing`; + await filesystem.mkdirp(existing); + await filesystem.chmod(existing, 0o755); + + await filesystem.mkdirp(directory, { mode: 0o750 }); + await filesystem.mkdirp(directory, { mode: 0o750 }); + await filesystem.mkdirp(existing, { mode: 0o750 }); + + expect((await filesystem.stat(directory)).mode).toBe(0o750); + expect((await filesystem.stat(`${temporaryDirectory}/home`)).mode).toBe(0o750); + expect((await filesystem.stat(existing)).mode).toBe(0o755); + } finally { + process.umask(previousUmask); + } + }); + it("writes a file exclusively, refusing the second writer rather than replacing", async () => { const filesystem = create(); const file = `${temporaryDirectory}/instance.json`; diff --git a/src/ports/filesystem.ts b/src/ports/filesystem.ts index 28ee11282..6cd3ccf32 100644 --- a/src/ports/filesystem.ts +++ b/src/ports/filesystem.ts @@ -44,6 +44,11 @@ export interface WriteFileExclusiveOptions { readonly mode?: number; } +export interface MkdirpOptions { + /** why: the bits the directory ends with whatever the umask, which can only remove them. */ + readonly mode?: number; +} + /** * What an un-followed `lstat` reports, narrowed to what ownership validation asks of a * path: what it really is, who owns it, and how exposed it is. Anything richer would be @@ -73,7 +78,13 @@ export interface Filesystem { contents: string, options?: WriteFileExclusiveOptions, ): Promise; - mkdirp(path: string): Promise; + /** + * why: a `mode` applies to every directory this call creates, parents included, and to none + * that is already there, so a caller can make its own directories owner-only (a gateway's + * `/gateway` makes `` too) without tightening one it does not own, or overriding + * the mode a user chose for a directory that exists. + */ + mkdirp(path: string, options?: MkdirpOptions): Promise; copyFile(from: string, to: string): Promise; /** * Moves a path in one step, so what it names becomes visible complete or not at all. @@ -106,6 +117,31 @@ export interface Filesystem { directorySize(path: string): Promise; } +// why: only the directories that are absent get the mode, so one that is already there keeps its own. +async function missingDirectories(path: string): Promise { + const missing: string[] = []; + for (let at = path; ; at = dirname(at)) { + try { + await stat(at); + return missing; + } catch (error: unknown) { + if (!hasCode(error, "ENOENT") || dirname(at) === at) throw error; + missing.unshift(at); + } + } +} + +// why: false means another process made it first, and its mode is left alone. +async function createDirectory(directory: string, mode: number): Promise { + try { + await mkdir(directory, { mode }); + return true; + } catch (error: unknown) { + if (hasCode(error, "EEXIST") && (await stat(directory)).isDirectory()) return false; + throw error; + } +} + export class NodeFilesystem implements Filesystem { async readFile(path: string): Promise { return readFile(path, "utf8"); @@ -182,8 +218,18 @@ export class NodeFilesystem implements Filesystem { } } - async mkdirp(path: string): Promise { - await mkdir(path, { recursive: true }); + async mkdirp(path: string, options: MkdirpOptions = {}): Promise { + if (options.mode === undefined) { + await mkdir(path, { recursive: true }); + return; + } + for (const directory of await missingDirectories(path)) { + if (await createDirectory(directory, options.mode)) { + // why: the umask can only take bits from `mkdir`'s mode, so a restrictive one would leave the + // directory short of what was asked; `chmod` sets exactly those bits, as the device root does. + await chmod(directory, options.mode); + } + } } async rename(from: string, to: string): Promise { @@ -367,7 +413,7 @@ export class MemoryFilesystem implements Filesystem { }); } - async mkdirp(path: string): Promise { + async mkdirp(path: string, options: MkdirpOptions = {}): Promise { this.#failIfDefined(path); let currentPath = ""; @@ -381,7 +427,9 @@ export class MemoryFilesystem implements Filesystem { const entry = this.#entries.get(currentPath); if (entry === undefined) { - this.#entries.set(currentPath, this.#newEntry({ kind: "directory" })); + const directory = this.#newEntry({ kind: "directory" }); + if (options.mode !== undefined) directory.mode = options.mode & 0o777; + this.#entries.set(currentPath, directory); } else if (entry.kind !== "directory") { throw new Error(`Cannot create directory over file: ${currentPath}`); } @@ -448,7 +496,7 @@ export class MemoryFilesystem implements Filesystem { kind: entry.kind === "file" ? "file" : "directory", size: entry.kind === "file" ? Buffer.byteLength(entry.contents) : 0, modifiedAtMs: entry.modifiedAtMs, - mode: entry.kind === "file" ? entry.mode : 0o755, + mode: entry.mode, identity: String(entry.identity), }; } @@ -691,8 +739,12 @@ function errnoError(code: string, message: string): NodeJS.ErrnoException { return error; } +function hasCode(error: unknown, code: string): boolean { + return typeof error === "object" && error !== null && "code" in error && error.code === code; +} + export function isMissingPathError(error: unknown): error is NodeJS.ErrnoException { - return typeof error === "object" && error !== null && "code" in error && error.code === "ENOENT"; + return hasCode(error, "ENOENT"); } function enoent(path: string): NodeJS.ErrnoException { diff --git a/src/ports/index.ts b/src/ports/index.ts index 7e72e9d46..2acd9795b 100644 --- a/src/ports/index.ts +++ b/src/ports/index.ts @@ -7,6 +7,7 @@ export { export type { PathDetails } from "./filesystem.js"; export { readPackageVersion } from "./package-version.js"; export { + DATA_DIRECTORY_MODE, daemonDataDirectory, type DaemonRole, type DaemonFlag, diff --git a/src/ports/paths.ts b/src/ports/paths.ts index 01c8f0672..58c430c39 100644 --- a/src/ports/paths.ts +++ b/src/ports/paths.ts @@ -72,6 +72,14 @@ export interface DaemonHome { readonly gatewayPresent: boolean; } +/** + * why: owner-only, because every socket peer counts as the same OS user and the directory's + * mode is what keeps other users away from the socket and the files beside it. Whoever creates + * the data directory gives it, and any parent it creates on the way, this mode: a gateway made + * first leaves `` owner-only too, which is the worker's data directory. + */ +export const DATA_DIRECTORY_MODE = 0o700; + export function daemonDataDirectory(root: string, role: DaemonRole): string { return role === "gateway" ? join(root, "gateway") : root; } diff --git a/src/ports/service-manager.test.ts b/src/ports/service-manager.test.ts index 786e38155..a1e4d564a 100644 --- a/src/ports/service-manager.test.ts +++ b/src/ports/service-manager.test.ts @@ -162,10 +162,21 @@ describe("LaunchdServiceManager (#830)", () => { } }); - it("install creates daemon.log at 0600 when it is missing and keeps one that exists", async () => { + it("install creates daemon.log at 0600 in a data directory it creates owner-only when they are missing, and keeps a log that exists", async () => { + // why: a directory the memory filesystem creates without a mode is already 0700, so the + // mode install asks for is read from the call it makes, not from the directory. + class RecordingFilesystem extends MemoryFilesystem { + readonly directories: Array<{ path: string; mode: number | undefined }> = []; + override async mkdirp(path: string, options?: { readonly mode?: number }): Promise { + this.directories.push({ path, mode: options?.mode }); + await super.mkdirp(path, options); + } + } const definition = definitionFor("worker"); - const created = await installed(definition); + const recording = new RecordingFilesystem(); + const created = await installed(definition, recording); expect((await created.filesystem.stat(definition.logPath)).mode).toBe(0o600); + expect(recording.directories).toContainEqual({ path: root, mode: 0o700 }); const existing = new MemoryFilesystem(); await existing.mkdirp(root); diff --git a/src/ports/service-manager.ts b/src/ports/service-manager.ts index 8027e87a7..e5d472c27 100644 --- a/src/ports/service-manager.ts +++ b/src/ports/service-manager.ts @@ -2,7 +2,7 @@ import { createHash } from "node:crypto"; import { dirname, isAbsolute, join } from "node:path"; import type { Filesystem } from "./filesystem.js"; -import type { DaemonRole } from "./paths.js"; +import { DATA_DIRECTORY_MODE, type DaemonRole } from "./paths.js"; import type { ProcessResult, ProcessRunner } from "./process-runner.js"; export type ServiceManagerName = "launchd" | "systemd"; @@ -204,7 +204,8 @@ function isExistsError(error: unknown): boolean { } async function ensureLog(filesystem: Filesystem, logPath: string): Promise { - await filesystem.mkdirp(dirname(logPath)); + // why: the log lives in the data directory, which a first install on a fresh machine creates. + await filesystem.mkdirp(dirname(logPath), { mode: DATA_DIRECTORY_MODE }); try { await filesystem.writeFileExclusive(logPath, "", { mode: 0o600 }); } catch (error: unknown) { diff --git a/website/src/content/docs/docs/worker/running-the-daemon.mdx b/website/src/content/docs/docs/worker/running-the-daemon.mdx index f03b23360..52cde0906 100644 --- a/website/src/content/docs/docs/worker/running-the-daemon.mdx +++ b/website/src/content/docs/docs/worker/running-the-daemon.mdx @@ -145,6 +145,8 @@ Everything the worker keeps is under `~/.simlock`: A gateway on the same machine keeps its own files in `~/.simlock/gateway`, with its own daemon. Commands reach it with `--gateway`, for example `simlock --gateway daemon status`. See [Setting up a gateway](/simlock/docs/gateway/setup). +Simlock creates `~/.simlock` (and `~/.simlock/gateway`) so only your user can open it, because anyone who can reach `daemon.sock` can lease devices as an agent. It does not change the permissions of a folder that already exists. If other users can open it, the daemon log says `data directory is accessible to other users (mode ); run chmod 700 ` at each start: run that command to close it. + To run a separate install on the same account, set `SIMLOCK_HOME` to another folder. Every command and the daemon it starts use that folder instead of `~/.simlock`. Keep the path short: the socket path must fit the system limit (104 bytes on macOS, 108 on Linux), and a longer one fails with `Point SIMLOCK_HOME at a shorter directory.`