diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 09d418c1..0fe95023 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -485,7 +485,7 @@ The general rule: **any new endpoint that turns a caller-supplied `sessionId` in - **Confinement is the read path's, plus write-only gates.** `findSessionOrFail` (ownership) → `validateSessionFilePath` (realpath + workspace boundary; escapes report as 404, same as reads) → sensitive-path + attachment-guard blocklists (403) → `.git/` subtree deny (403 — `.git/hooks/*` is code execution) → extension **allowlist** (400; `svg` and `env` deliberately excluded from `EDITABLE_EXTENSIONS`). ⚠️ **Dotenv is the one sensitive family the editor opens**: both the gate (`assertEditableTarget`) and the `editable` flag on the plain preview call `isBlockedAttachmentPath(..., { allowDotenv: true })`, which skips only `DOTENV_PATTERNS` in `sensitive-path.ts`, and `isEditableFileName` accepts `.env`, `.env.*` and `*.env` by NAME. ⚠️ Never add `env` to `EDITABLE_EXTENSIONS`: it is also `TEXT_ATTACHMENT_EXTENSIONS`, so it would let cross-workspace attachments read any `prod.env` on the host, which no sensitive pattern matches. The reason is that the plain preview and `file-raw` never consult the blocklist (realpath containment is their control), so refusing the edit protected nothing the preview had not already shown. Every other credential pattern still refuses, so does a dotenv file inside a blocked tree (`.ssh/.env`, a config backup), and the data dir's own `.env` (the `codeman attach` login fallback) has its own entry in `SENSITIVE_PATTERNS` so a session whose workspace is `$HOME` cannot open it. Downloads, attachments and the path picker keep the full list. Keep the two call sites in step. Accepted limits: `/\.env\./` also matches a FOLDER named `.env.`, so files under one become editable too (the preview already shows them); and like every `.codeman[^/]*` entry, the data-dir entry misses a data dir moved by `CODEMAN_DATA_DIR` to a folder not named `.codeman*`, or reached through a symlinked `~/.codeman`, which already left that dir's `users.json`/`settings.json` editable when it sits inside a workspace. ⚠️ **There is no `O_CREAT` anywhere in the handler** — that absence is what makes "edit-in-place only, never create" a structural property instead of a convention. Do not add a create path without treating it as a new security surface. - **A truncated buffer must never become an edit buffer.** The plain preview truncates to `lines` (default 500); saving such a buffer would silently delete everything past the cut, and the hash check cannot catch it (the loaded prefix hashes differently from the full file, which reads as an ordinary conflict at best). `edit=1` therefore never truncates — it 413s over `MAX_EDITABLE_BYTES` (512KB) instead — and the frontend always re-fetches with `edit=1` before swapping in the textarea, even though the preview already holds content. - **Concurrency is optimistic by content hash, not mtime.** The client echoes the sha256 it loaded (`baseHash`); mismatch → 409 CONFLICT (plain envelope — the error arm carries no data; the client re-fetches `edit=1` for fresh state) unless `force:true`. mtime alone is wrong: agents rewrite files within one timestamp tick. -- **Writes are `wx` temp + `fchmod` + `fsync` + `rename` in the target's directory.** `wx` cannot follow a pre-existing symlink and `rename()` replaces (not follows) a symlink final component, which closes the validate-then-write TOCTOU window; `fchmod` because `open()`'s mode argument is masked by the umask; a symlink whose target is *inside* the workspace is deliberately written through (validation returns the realpath). Trade-off (same as vim): the inode changes, so hardlinks keep old content. +- **Writes are `wx` temp + `fchmod` + `fsync` + `rename` in the target's directory.** `wx` cannot follow a pre-existing symlink and `rename()` replaces (not follows) a symlink final component, which closes the validate-then-write TOCTOU window; `fchmod` because `open()`'s mode argument is masked by the umask; before it a best-effort `fchown` back to the old file's user and group (group alone when the user cannot be set), because the rename publishes a NEW file that would take the server's own group, and a `0640` `.env` owned `you:www-data` came back `you:you`; a symlink whose target is *inside* the workspace is deliberately written through (validation returns the realpath). Trade-off (same as vim): the inode changes, so hardlinks keep old content. - **Corruption guards**: NUL-sniff + UTF-8 **round-trip compare** (`Buffer.from(buf.toString('utf8'), 'utf8').equals(buf)`) refuse binary and non-UTF-8 files — decoding latin-1 yields U+FFFD replacements and writing those back destroys the original bytes. EOL is detected server-side and re-applied on save because a `