Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
723a386
tm-config-audit-j09 Add addRoot/removeRoot/getRoots to IWorktreeApi
0101 Jun 19, 2026
c27c164
tm-config-audit-bjo Global-config roots read/write with write lock
0101 Jun 19, 2026
3767bb3
tm-config-audit-9ow Implement addRoot/removeRoot/getRoots endpoints (…
0101 Jun 19, 2026
5aa5cb3
tm-config-audit-3ng Server startup root resolution + orphan migration
0101 Jun 19, 2026
56da367
tm-config-audit-am6 CLI add/remove/roots subcommands
0101 Jun 19, 2026
dceafb1
tm-config-audit-krj Strip roots logic from treemon.ps1 + migrate .tre…
0101 Jun 19, 2026
e42e5e3
tm-config-audit-l8z Update docs for global roots + tm add/remove
0101 Jun 19, 2026
8462804
tm-config-audit-rf1 Fix empty-vs-absent worktreeRoots conflation that…
0101 Jun 19, 2026
28af7e6
tm-config-audit-rf2 Fix partial-batch tm add/remove suppressing the p…
0101 Jun 19, 2026
091c5f0
tm-config-audit-rf3 Read-only API should return Error (not Ok) for ad…
0101 Jun 19, 2026
40bfe85
tm-config-audit-rf4 Refactor treemon.ps1: extract Restart-ServerIfRun…
0101 Jun 19, 2026
e2496b0
tm-config-audit-rf5 Hoist duplicated withTempConfigDir test helper in…
0101 Jun 19, 2026
95b237e
tm-config-audit-rf6 F# idiom nits in new roots code: _.GetString() sh…
0101 Jun 19, 2026
610384c
Use modern indexer syntax in WorktreeRootsConfigTests
0101 Jun 22, 2026
7c01936
Merge remote-tracking branch 'origin/main' into config-audit
0101 Jun 22, 2026
5403b31
Merge branch 'main' into config-audit
0101 Jun 22, 2026
f911a5c
Address review findings: pure roots resolver, lossless legacy migrati…
0101 Jun 23, 2026
cd58f95
Document remoting CSRF/Origin hardening as future work
0101 Jun 23, 2026
5309ed3
focused-review: inherit orchestrator model for architecture/bugs conc…
0101 Jun 23, 2026
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
8 changes: 5 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ npm install
dotnet test src/Tests/Tests.fsproj # all tests
dotnet test src/Tests/Tests.fsproj --filter "Category=Fast" # fast suite (<60s)
dotnet test src/Tests/Tests.fsproj --filter "Category=Unit" # unit tests only
.\treemon.ps1 dev "Q:\code\AITestAgent" # dev mode
.\treemon.ps1 dev "Q:\code\AITestAgent" # dev mode (path optional; omit to use global config roots)
.\treemon.ps1 deploy # build + restart production
.\treemon.ps1 add "Q:\code\OtherProject" # add a root to config
.\treemon.ps1 remove "Q:\code\OtherProject" # remove a root from config
.\treemon.ps1 add "Q:\code\OtherProject" # add a watched root (shim for 'tm add'; restarts prod if running)
.\treemon.ps1 remove "Q:\code\OtherProject" # remove a watched root (shim for 'tm remove'; restarts prod if running)
```

## F# Style Guide
Expand Down Expand Up @@ -85,6 +85,8 @@ This project uses [focused-review](https://github.com/0101/focused-review) for a

`treemon.ps1` manages the application lifecycle: `dev`, `deploy`, `start`, `stop`, `restart`, `status`, `log`, `add`, `remove`.

Watched worktree roots live in the global config (`~/.treemon/config.json` → `worktreeRoots`), written only by the server. `start`/`dev` no longer need a path — omit it to use the global roots (an empty list is valid). Manage roots live with the `tm` CLI — `tm add <path>...`, `tm remove <path>...`, `tm roots` — or the `treemon.ps1 add`/`remove` shims, which call `tm` and restart production when it is running. Changes persist immediately and apply on the next server (re)start. See `docs/spec/worktree-monitor.md` (Multi-Repo).

## Tech Stack

- **Client**: F# with Fable (compiles to JS), Feliz for React bindings, Vite for bundling
Expand Down
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ npm install

Open http://localhost:5000 — install as a PWA from the browser for a native app experience.

The roots you pass to `start` are saved to the global config (`~/.treemon/config.json` → `worktreeRoots`, written by the server), so afterwards `start`, `restart`, and `dev` no longer need a path — omit it to use the saved roots. Manage roots live with the `tm` CLI (`tm add`, `tm remove`, `tm roots`) or the `.\treemon.ps1 add`/`remove` shims; changes apply on the next server restart.

### Managing the server

```powershell
Expand Down Expand Up @@ -79,6 +81,9 @@ tm launch --path C:\code\my-project --fix-tests # fix failing tests
tm launch --path C:\code\my-project --create-pr # create a pull request
tm new --repo C:\code\my-project --branch feature/foo # create worktree
tm worktrees # list all worktrees
tm add C:\code\my-project # watch a root (applies on next server restart)
tm remove C:\code\my-project # stop watching a root
tm roots # list watched roots
```

All commands accept `--port` (default: 5000, env: `TREEMON_PORT`). You can also run `.\tm.ps1` directly from the repo root without installing.
Expand Down
1 change: 1 addition & 0 deletions docs/spec/future/code-improvements.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ its own worktree. This file is the entry point; detailed designs live in their o
| 3 | **Canvas roadmap items** — follow-on canvas-pane enhancements | `docs/spec/future/canvas-roadmap.md` | Idea |
| 4 | **Process: guard against spec drift** — a lightweight check (or review rule) that flags `Key Files` references to moved/renamed modules so docs can't silently rot after refactors | — | Idea |
| 5 | **Survey other large modules** — apply the same view/state/update extraction lens to the next-largest files (server-side `RefreshScheduler.fs`, `WorktreeApi.fs`, etc.) if they mix concerns | — | Idea (needs investigation) |
| 6 | **Remoting CSRF / Origin hardening** — pipeline-level Origin/Referer (and optional custom-header) check so a cross-origin browser page can't drive the unauthenticated loopback Fable.Remoting API (covers the dangerous pre-existing process-launching endpoints, not just watched-roots) | `docs/spec/future/remoting-csrf-hardening.md` | Idea (from focused-review) |

> Add new candidates here as they surface (often from focused-review findings). Keep the list
> honest: remove ones that turn out not to be worth it, and record why in the relevant spec.
Expand Down
138 changes: 138 additions & 0 deletions docs/spec/future/remoting-csrf-hardening.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
# Remoting CSRF / Origin Hardening

Status: **Future / Deferred** — design only. Surfaced by focused-review (finding C-13 /
A-13, *Confirmed · Low*) on the `tm-config-audit` branch. Not implemented.

The branch that surfaced this only **added** three endpoints (`addRoot`/`removeRoot`/
`getRoots`) to an already-exposed surface; it did not create the gap. This spec is about
hardening the *whole* remoting surface, not those three endpoints.

## Problem Statement

The server exposes `IWorktreeApi` over Fable.Remoting with **no authentication and no
Origin/CSRF validation**, bound to loopback. A malicious web page open in the operator's
browser can therefore issue cross-origin `POST http://localhost:5000/IWorktreeApi/<method>`
requests that the server executes as if they were same-origin — a classic localhost CSRF.

The newly-added watched-roots endpoints are the *least* dangerous members of this surface.
The same handler already exposes state-changing, process-launching endpoints that share the
identical exposure — e.g. `launchSession`, `launchAction`, `resumeSession`,
`sendCanvasMessage` (builds and launches a coding-tool CLI process), `createWorktree`,
`deleteWorktree`, `openTerminal`, `openEditor`. If localhost CSRF is in scope at all, those
are the high-value targets; fixing it once at the pipeline protects all of them.

### Why the usual mitigations don't close it (verified)

- **No control exists.** `buildRemotingHandler` (`src/Server/Program.fs`) is
`createApi → fromValue → withErrorHandler → buildHttpHandler` and the Saturn pipeline is
`use_router / url / use_static / use_gzip` — no `use_cors`, no Origin/Referer middleware,
no CSRF token, no auth. A tree-wide grep for `cors|origin|access-control|referer|
x-requested-with` in `src/Server` finds only an unrelated git remote and same-origin
client JS.
- **Loopback bind is not a defense for this threat model.** The malicious page runs in a
browser *on the same machine*, which can reach `http://localhost:5000`. The default port
(`5000`) is well-known, so the attacker need not guess it.
- **The CORS-preflight "mitigation" does not apply.** A cross-origin `fetch` with
`Content-Type: application/json` would be a *non-simple* request and trigger a preflight
the no-CORS server fails — but Fable.Remoting.Server (verified against the resolved
`5.42.0`) does **not** enforce `application/json`. Its dispatcher requires `POST` for
argument-taking methods, then the only content-type branch is `multipart/form-data`; every
other type — including `text/plain` — falls through to
`StreamReader.ReadToEndAsync()` → `JsonConvert.DeserializeObject<JToken>` and is dispatched.
A cross-origin `text/plain` POST is a CORS **simple request** (no preflight), so its
JSON-array body reaches the method. The attacker can't read the response (that *is*
CORS-blocked), which is irrelevant for a state-changing call.

### Impact ceiling (why this is Low, not High)

- The exposure is **pre-existing** — this is a latent design property of the remoting
handler, not a regression.
- For the roots endpoints specifically: `removeRoot` is a blind guess (the attacker must
supply the exact canonical watched path and can't enumerate it, since `getRoots`' response
is CORS-blocked from being read); `addRoot` only accepts a directory that already exists on
the victim's disk and its effect is deferred to the next server restart. Worst realistic
outcome there is polluting/clearing one config key or a wasteful filesystem walk on
restart — integrity/availability, never code execution.
- The real value of acting is the **process-launching** endpoints, where a forged call has
materially higher impact.

## Goals

- A cross-origin browser page **cannot** invoke any `IWorktreeApi` method.
- The fix is applied **once, at the remoting pipeline**, so every method (current and future)
is covered without per-endpoint changes.
- **No friction for legitimate callers**: the same-origin web client and the `tm` CLI keep
working unchanged (or with a single, central client change).
- Production keeps its well-known port and loud-on-conflict behavior (see
`docs/spec/future/port-management.md`); this is orthogonal to where it binds.

## Technical Approach

Add a small middleware *in front of* the Fable.Remoting handler in `buildRemotingHandler`
(or wrapping it in the Saturn pipeline) that rejects forged cross-origin requests before they
reach any method.

**Primary control — Origin/Referer allowlist (no client change):**

- If an `Origin` header is present and is **not** same-origin (`http://localhost:{port}` /
`http://127.0.0.1:{port}`), reject with `403`.
- If `Origin` is absent, fall back to a `Referer` same-origin check.
- Requests with **neither** header (the `tm` CLI via `HttpClient`, server-to-server) are
allowed — non-browser clients don't forge CSRF.

Browsers attach `Origin` to cross-origin POSTs (and to same-origin POSTs), so the legitimate
web client passes and the malicious page is blocked, with **zero client changes**. This is
the lowest-friction option.

**Defense in depth — required custom header (one central client change):**

- Require a non-simple custom request header (e.g. `x-requested-by: treemon`) on every
remoting call. A CORS *simple request* cannot set a custom header without triggering a
preflight the no-CORS server will fail, so cross-origin pages are blocked at the browser.
- This needs the Fable.Remoting client to be configured to send the header on every call
(one place, via the client's header/route customization), and the `tm` CLI's `HttpClient`
to add the same header.

Requiring `Content-Type: application/json` is **necessary but not sufficient** on its own
(confirmed the library doesn't enforce it and `text/plain` bypasses it), so it is not the
control — the Origin check and/or custom-header are.

Recommended: ship the **Origin/Referer allowlist** first (smallest, no client change), and
optionally add the custom-header check as belt-and-suspenders.

## Decisions

- **Fix at the pipeline, not per-endpoint.** The vulnerability is a property of the shared
handler; a single middleware covers the dangerous pre-existing endpoints too. Per-endpoint
guards would be incomplete and easy to forget on the next method added.
- **Origin/Referer over CSRF tokens.** A stateless Origin allowlist needs no session, token
store, or client plumbing — appropriate for a single-user localhost tool. CSRF tokens would
be over-engineered here.
- **Allow header-less requests.** The non-browser `tm` CLI legitimately sends no `Origin`;
CSRF is exclusively a browser-driven attack, so absence-of-Origin is safe to allow and
avoids breaking the CLI.
- **`application/json` enforcement is not the control.** Kept only as optional extra
hardening; the verified `text/plain` fall-through means it can't stand alone.

## Key Files

- **Server**: `src/Server/Program.fs` — `buildRemotingHandler` (add the Origin/Referer
middleware here) and the Saturn app builder (`use_router/url/use_static/use_gzip`); the
loopback `serverUrl` + default port feed the same-origin allowlist value.
- **Shared contract (context)**: `src/Shared/Types.fs` — `IWorktreeApi` (all methods covered
by the pipeline fix; the roots methods that surfaced this are `addRoot`/`removeRoot`/
`getRoots`).
- **Implementations (context)**: `src/Server/WorktreeApi.fs` — `addRootToConfig` /
`removeRootFromConfig` / `writeWorktreeRoots` and the pre-existing process-launching members.
- **Client (only if custom-header option is chosen)**: the Fable.Remoting client setup in
`src/Client/` (central header/route config) and the `tm` CLI `HttpClient` in
`src/Cli/Program.fs`.

## Verification

- A cross-origin `text/plain` POST to `removeRoot`/`addRoot` (and a process-launching method)
is rejected `403` once the middleware is in place.
- The same-origin web client and `tm add` / `tm remove` continue to work unchanged (Origin
option) or after the single client header change (custom-header option).
- Full suite stays green (Unit + Fast + E2E); E2E exercises the same-origin client path end
to end.
5 changes: 4 additions & 1 deletion docs/spec/worktree-monitor.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@

### Multi-Repo

- Server accepts multiple root paths as positional CLI args
- Watched roots resolve at server startup by priority: CLI args → the global `worktreeRoots` key in `~/.treemon/config.json` → a one-time import of the legacy orphan `~/.treemon/roots.json`. With roots configured, `treemon.ps1 start`/`dev` no longer require a path; with no args the server uses the global config (an empty list is valid, as in demo mode). A *present* `worktreeRoots` key — even an explicit empty list — is authoritative and never repopulated; the server persists a resolved set only when the key is *absent* (fresh install / migration), so curating every root away stays sticky across restarts.
- Roots are managed live through the `tm` CLI — `tm add <path>...` (validates the path exists, normalizes it, no-op if already watched), `tm remove <path>...` (errors on an unknown path; removing the last root is allowed), and `tm roots` (list). All three are online-only (require the running server). The server is the single, serialized writer of `config.json`; changes persist immediately and take effect on the next server (re)start. The `treemon.ps1 add`/`remove` shims call `tm` and then restart the production server when it is running.
- Roots are a per-machine singleton: dev and prod instances on the same machine share one global list. Legacy stores migrate then delete losslessly — `treemon.ps1` migrates a legacy `.treemon.config` (PowerShell-written, plural `WorktreeRoots` or the older singular `WorktreeRoot`) and the server migrates the orphan `roots.json`, each removed only after its roots are safely persisted (a parse failure or unmigrated content is preserved with a warning, never silently dropped).
- Each root is an independent section — cards never mix across repos
- Scheduler picks most-overdue task globally across all repos
- Branch events scoped by `{repoId}/{branch}` to prevent cross-repo collisions
Expand Down Expand Up @@ -256,6 +258,7 @@ After the burst, `lastRuns` is pre-populated and the normal sequential loop take
- net9.0 (not net10.0): Fable 4.28.0 FCS hangs with .NET 10 preview SDK
- Windows Terminal per-window tracking via HWND: tabs aren't reliably addressable, one window per worktree is simple and predictable
- Upstream remote auto-detection over config-only: `upstream` remote name is the universal convention for fork workflows; config override available for non-standard setups
- Watched roots are server-owned and restart-to-apply (not live-updated): `tm add`/`remove` persist to the global config and take effect on the next server (re)start (the `treemon.ps1` shims trigger it when prod is running). Chosen for simpler code — no per-root scheduler-state machinery; live application remains a clean future extension. The server is the single writer of `config.json` (with an internal write lock); the online-only CLI never writes config files, which removes the cross-process clobber hazard.

## Related Specs

Expand Down
2 changes: 1 addition & 1 deletion review/concerns/architecture.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
type: concern
models: [gemini-3-pro-preview, gpt-5.3-codex, claude-opus-4.6]
models: [gemini-3-pro-preview, gpt-5.3-codex, inherit]
priority: standard
---
# Architecture Reviewer
Expand Down
2 changes: 1 addition & 1 deletion review/concerns/bugs.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
type: concern
models: [gemini-3-pro-preview, gpt-5.3-codex, claude-opus-4.6]
models: [gemini-3-pro-preview, gpt-5.3-codex, inherit]
priority: high
---
# Bug Finder
Expand Down
73 changes: 73 additions & 0 deletions src/Cli/Program.fs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,76 @@ let worktreesCmd =
setAction handler
}

/// Folds a per-path root operation (add/remove) into a tri-state exit code:
/// 0 = all paths succeeded, 1 = all paths failed, 2 = partial success.
/// A partial batch returns 2 (not 1) because the paths that succeeded WERE persisted
/// server-side immediately, before a later path failed validation. The treemon.ps1 shims
/// restart prod on 0 OR 2 so those persisted roots actually apply, while a full failure (1)
/// skips the restart (don't bounce prod when nothing changed). Any failure still yields a
/// non-zero exit, so callers/scripts can still detect rejected paths.
let foldRootResults (verb: string) (op: string -> Async<Result<unit, string>>) (paths: string[]) : int =
let anySuccess, anyFailure =
((false, false), paths)
||> Array.fold (fun (anySuccess, anyFailure) path ->
match op path |> Async.RunSynchronously with
| Ok() -> printfn $"✓ %s{verb} %s{path} (applies on next server restart)"; (true, anyFailure)
| Error e -> eprintfn $"Error: %s{e}"; (anySuccess, true))

match anySuccess, anyFailure with
| true, true -> 2
| false, true -> 1
| _ -> 0

let addCmd =
let handler (paths: string[], port: int option) =
withPort port (fun port ->
// One tryCallServer for the whole batch so the "server is not running" message
// (and the connection check) happens once, not once per path.
tryCallServer port (fun api -> foldRootResults "Added" api.addRoot paths))

command "add" {
description "Add one or more worktree roots to watch (applies on next server restart)"

inputs (
argument<string[]> "paths" |> arity Arity.OneOrMore |> desc "Worktree root path(s) to watch",
optionMaybe<int> "--port" |> desc "Server port (default: 5000, env: TREEMON_PORT)"
)

setAction handler
}

let removeCmd =
let handler (paths: string[], port: int option) =
withPort port (fun port ->
tryCallServer port (fun api -> foldRootResults "Removed" api.removeRoot paths))

command "remove" {
description "Remove one or more worktree roots (applies on next server restart)"

inputs (
argument<string[]> "paths" |> arity Arity.OneOrMore |> desc "Worktree root path(s) to stop watching",
optionMaybe<int> "--port" |> desc "Server port (default: 5000, env: TREEMON_PORT)"
)

setAction handler
}

let rootsCmd =
let handler (port: int option) =
withPort port (fun port ->
tryCallServer port (fun api ->
match api.getRoots() |> Async.RunSynchronously with
| [] -> printfn "No worktree roots configured."; 0
| roots ->
roots |> List.iter (fun root -> printfn $"%s{sanitizeForTerminal root}")
0))

command "roots" {
description "List the configured worktree roots"
inputs (optionMaybe<int> "--port" |> desc "Server port (default: 5000, env: TREEMON_PORT)")
setAction handler
}

[<EntryPoint>]
let main argv =
rootCommand argv {
Expand All @@ -224,4 +294,7 @@ let main argv =
addCommand launchCmd
addCommand newCmd
addCommand worktreesCmd
addCommand addCmd
addCommand removeCmd
addCommand rootsCmd
}
Loading
Loading