Skip to content
Closed
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
57 changes: 57 additions & 0 deletions apps/documentation/src/content/docs/decisions/0020-file-lock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
---
title: ADR-0020 — Cross-process file lock: O_EXCL sidecar with guarded reclaim, async wait, no dependency
---

# ADR-0020 — Cross-process file lock: `O_EXCL` sidecar with guarded reclaim, async wait, no dependency

**Status:** Proposed
**Date:** 2026-10-12
**Ticket:** [NIW2-313](https://linear.app/niway/issue/NIW2-313)
**Implementation:** not implemented; design in [Core hardening — design spec](../specs/core-hardening.md) §5

## Context

Two independent processes write the same files: the CLI's control plane (`laqi`) and the MCP server (`laqi mcp`), both through `Project` in `@laqi/core`. They write mock files and `.laqi/state.json`. [ADR-0004](/decisions/0004-state-outside-git/) put state in that file, and [ADR-0006](/decisions/0006-mcp-server/) promised that MCP works whether or not the server is running, so there is no single process that could own the file.

`atomic-file.ts` on `main` already has a lock: a sidecar `<file>.lock` opened with `O_CREAT|O_EXCL`, the owner's pid inside, reclaimed when the pid is dead. The audit (core.md Q1, Q2, Q3, Q5, Q6, P3) found four defects in it:

1. **Reclaim race.** Two waiters that both judged a lock dead both `rmSync` it; the second removes the _new_ owner's lock. Two processes end up in the critical section and one write is lost — the very failure the lock exists to prevent.
2. **Event-loop stall.** Waiting is `Atomics.wait` on the main thread, up to 5 s, so the HTTP server, the SSE stream and the panel freeze while another process holds the lock.
3. **Silent failure.** `StateStore.write` discards the lock's outcome; a throw inside the critical section escapes the Result-typed API.
4. **Hygiene.** A failed token write leaks the descriptor; a readable pid from another host or pid namespace is judged dead at once; a recycled pid is "alive" forever.

The question is not whether to fix these but _what to replace the lock with_: a library, a kernel lock, a single-writer architecture, or a repaired version of what exists.

## Decision

**Keep the `O_EXCL` sidecar lock, repair it, and add no dependency.** Concretely:

- Lock content is an owner token `pid:hostname:random64`. Liveness is judged by pid only for tokens from this host; foreign hosts are judged by age; a token this process currently holds is alive by definition.
- Waiting is asynchronous (`setTimeout` polling every 10 ms, 5 s deadline). The critical section (`work()`) stays synchronous.
- A lock is removed by someone other than its owner only under a short-lived guard file (`<file>.lock.reclaim`, also `O_EXCL`), and only after re-reading it and matching content, inode and mtime against the snapshot that was judged dead. The owner's release takes the same guard and removes the file only if it still carries its own token.
- Every failure — cannot open, timed out, `work()` threw — is a `{ ok: false, error }` outcome. Callers surface it (503 in the control plane, `isError` in MCP).

## Alternatives rejected

- **`proper-lockfile`.** Gives up the instant reclaim of a crashed owner (its `stale` minimum is 5 s, default 10 s), adds three transitive dependencies to the published bundle, and its README lists "a lockfile is manually removed and someone else acquires the lock right after" as an undetected case — the same race class as defect 1. Its mtime-refresh guard is for long-held locks, which laqi never has.
- **`flock`/`fcntl` range locks.** Would give kernel-enforced release on crash, but Node core does not expose them and shipping a native addon (`fs-ext`) in an `npx` CLI is not acceptable.
- **A single writer process** (MCP forwards writes to the running server). Would remove the lock entirely, but breaks ADR-0006's "works with laqi stopped".
- **`mkdir` as the primitive with pid file inside.** Equivalent to `O_EXCL` on local filesystems and better on NFSv2; gives up nothing but also fixes nothing by itself (the reclaim race is identical). Not worth changing the on-disk shape.
- **Keep synchronous waiting with a shorter timeout.** Gives up nothing in API churn, but still freezes requests for the shortened time.

## Consequences

- Every function that takes the lock becomes `async`; reads stay synchronous. `@laqi/core` is private, so the only consumers are in this repository (`serve.ts`, `control-plane-app.ts`, `mcp/server.ts`), and the compiler lists every call site.
- Two short-lived files can appear next to a written file: `<file>.lock` and `<file>.lock.reclaim`. Both are swept by the stale window after a crash. They are not `.json`, so the loader ignores them; `.gitignore` templates should list `*.lock`.
- The residual race is: a lock judged dead _wrongly_ (recycled pid, foreign host within the stale window) whose owner is slow. The guard closes the owner's release path too, so even then the owner cannot delete the reclaimer's lock; the owner's own in-flight write may still race the reclaimer's. This needs a crash-like misjudgement plus sub-millisecond timing, and is documented in the code.
- `O_EXCL` on NFSv2 is unreliable. laqi assumes a local project directory, the same assumption chokidar makes for hot reload.

## Non-goals

- Locking across machines sharing one project directory. Age-based reclaim makes it _survivable_, not correct.
- Protecting against a human editing a mock file in their editor while laqi writes it. That is the writer's revision check (`responseRevision`), not the lock.
- Long critical sections. If one ever appears, mtime refresh must be added before raising the stale window.

## Reopen condition

Node core gains a portable advisory lock (`fs.flock` or equivalent) that releases on process death, or laqi gains a daemon that owns the project directory. Either removes the need for userland reclaim and this ADR should be superseded.
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
title: ADR-0021 — Mock files are never read through symbolic links
---

# ADR-0021 — Mock files are never read through symbolic links

**Status:** Proposed
**Date:** 2026-10-12
**Ticket:** [NIW2-313](https://linear.app/niway/issue/NIW2-313)
**Implementation:** not implemented; design in [Core hardening — design spec](../specs/core-hardening.md) §1

## Context

The loader walks the mocks folder and reads every `*.json` it finds. On `main` it skips dotfiles and does not recurse into symlinked directories (`Dirent.isDirectory()` is false for a link), but it does read a symlinked _file_, and in single-file mode it reads `laqi.json` even when that is a link. When the target is not JSON, the parse error carries a four-line excerpt of the file, and that excerpt is served by `/api/status`, shown in the panel and returned to coding agents by the MCP `list_endpoints` tool.

So a repository can ship `laqi/leak.json -> ~/.aws/credentials` or `laqi.json -> .env`, and the first lines of the victim's secrets reach every laqi output the moment they run `npx laqi`. The agent channel makes it an exfiltration path, not just a display bug. This is CWE-59 (improper link resolution before file access), and [ADR-0006](/decisions/0006-mcp-server/) already requires MCP to be "strictly scoped to the mocks directory" — for writes. Reads had no equivalent rule.

Other dev servers draw the same line: Vite's `server.fs` refuses to serve outside the workspace root by default and denies `.env`, `.git` and key files even inside it.

## Decision

**A symbolic link found inside the mocks area is never followed.** Neither a linked file nor a linked directory is read; in single-file mode a `laqi.json` that is itself a link is refused. Each link is reported as a load error that names the link's path and a fixed message (`symbolic links are not loaded — copy the file into the mocks folder instead`), with no excerpt and no target path.

The path the user _configured_ (`--dir`, `--file`, or the config defaults) may itself be a symlink: the user chose it, and the writer already supports a mocks directory reached through a link.

Excerpts are built only for regular files that passed this rule, and are clipped to 200 characters per line.

## Alternatives rejected

- **Follow a link when its real path stays inside the real mocks root.** Gives up the strictness for a feature nobody asked for: it would let a monorepo share one mock file between two packages by link, but the write path renames over links (the shared file would silently become a private copy on the first panel edit), directory links would still not be followed, and the check-then-read window needs `O_NOFOLLOW`, which Node lacks on Windows.
- **Follow links but suppress the excerpt for targets outside the root.** Gives up nothing visible, but still parses the foreign file (its top-level keys appear in per-key error messages) and still loads a foreign file that happens to be valid JSON as endpoints.
- **Do nothing and document "do not clone untrusted repos".** Gives up the one property the audit asked for — no content from outside the mocks root in any output — for a tool whose MCP server is meant to be run against repositories the user did not write.

## Consequences

- A project that linked individual mock files into `laqi/` loses those endpoints and gets one error per link, with the remedy in the message. No such project is known; the change is listed under "security" in the changelog.
- The loader's `LoadError` gains an optional `kind: 'symlink'` (and `'too-large'` for the size cap). Consumers that print `message` need no change.
- The existing C2 loader test, which used a dangling link to simulate a file vanishing between scan and read, must be rewritten with a regular file.

## Non-goals

- Hard links. A hard link _is_ the file; no link resolution happens and nothing can be done about it in userland.
- Bind mounts, junctions and other OS-level aliases outside the symlink model.
- Links in the schema-source root (`schemaSources.root`). Those are governed by `resolveSourcePath`, which follows links but requires the real path to stay inside the source root and refuses hidden paths; see the spec §2.

## Reopen condition

A documented user layout that needs linked mock files (for example a monorepo sharing one `laqi/` across packages) _and_ a write path that preserves links instead of renaming over them. Then the "follow if inside the real root" alternative becomes the right answer, implemented with `O_NOFOLLOW` where available.
Loading
Loading