Skip to content

feat(furrow): reach a run's live workspace from anywhere - #130

Open
AbirAbbas wants to merge 35 commits into
mainfrom
feat/furrow-workspace-handle
Open

feat(furrow): reach a run's live workspace from anywhere#130
AbirAbbas wants to merge 35 commits into
mainfrom
feat/furrow-workspace-handle

Conversation

@AbirAbbas

@AbirAbbas AbirAbbas commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

A build's workspace is currently only reachable if you happen to be on the same machine as the node. This makes it reachable from anywhere, so the harness that started a run can read the files while the run is still going — including the uncommitted edits, untracked files and dev state a git push would never carry.

Mirroring is opt-in: set SWE_FURROW_ENABLED=1 (or true/yes/on) to turn it on. Unset, 0, or anything unrecognised leaves it off, and the manifest ships the default as 0. A mirror is a byte-exact second copy of the whole build workspace — untracked files and any secrets the coder wrote there included — so an operator has to ask for it and accept the disk it costs.

With it on, nothing else has to know it is there. Availability is discovered, never configured: a build's result carries a workspace_handle when the mirror is working and simply does not carry one when it is not. Every unavailable path — feature off, no binary, no .git, attach or publish failing — degrades to exactly today's behaviour.

What is here

internal/furrow — resolves a vendored furrow binary the same way internal/pro resolves the engine, attaches a build's clone, republishes at each DAG-level boundary and at completion, and keeps a registry mapping the control-plane run ID to the workspace on disk. That mapping is the thing that was missing: SWE-AF knew its workspaces only by a locally generated build ID, so nothing outside the process could name one. An hourly sweeper bounds what keeping them costs, because furrow never prunes a remote on its own:

  • AgeSWE_FURROW_TTL_HOURS, default 72. A mirror whose last publish is older is retired.
  • DiskSWE_FURROW_MAX_GB, default 20, covering the client store and every run's remote together. Over budget, the sweeper retires the least recently published mirrors, and Attach refuses to start new ones until there is room. SWE_FURROW_MAX_GB=0 means unlimited — no cap, no budget eviction. Budget eviction never touches a mirror that published within the last hour, so reclaiming space cannot delete a running build's workspace; it degrades to refusing new mirrors instead.

cmd/furrowd + cmd/furrow-dial — the transport. furrowd serves one run's encrypted blob store over TLS by spawning furrow __remote behind a token check; furrow-dial stands in as FURROW_SSH_COMMAND on the caller's side so a stock furrow clone reaches it with no patches to furrow.

Deliberately not sshd: the cloud image runs the control plane and its agent nodes in one container as root, so an ssh key there is whole-box access to every run and every secret on the volume. A furrowd token unlocks a single run's data root, and the payload is ciphertext either way.

get_workspace_handle — returns a run's handle on demand, or {"available": false}. It is registered only on a node where mirroring is actually on.

It performs no per-caller authorization: anything that can reach the node and name a run gets an answer. So the two secrets in a handle are withheld by default and the result says "secrets_redacted": true — the recovery key (which decrypts that workspace) and the transport token (which authenticates to furrowd read-write). What remains is the remote, namespace, handle version and on-node repo path: enough for a caller that already shares the filesystem, and enough to see that a mirror exists. SWE_FURROW_EXPOSE_SECRETS=1 returns them in full, and is appropriate only on a single-tenant cluster where every caller is already trusted with the workspace contents.

Three bugs that only a real binary could find

The unit tests inject a fake exec and assert argv, which pins what we mean to run. These are all things a fake happily accepted:

  • furrow snap takes -m; there is no --label.
  • The recovery key comes back as key_hex, not key.
  • .furrowpolicy lines must be exclude <subtree>. Bare names make furrow reject the file, watch then exits non-zero, and the mirror is silently off for every build with one debug line to show for it — the worst failure mode a feature like this can have.

internal/furrow/integration_test.go now drives the real binary end to end (attach → publish → materialize elsewhere → diff) and skips when furrow is not installed, so the CLI contract is checked rather than assumed. The exact policy bytes are pinned in a unit test too, since CI has no furrow binary.

.furrowpolicy also joins .artifacts/ and .worktrees/ in .git/info/exclude — it lands in the repository the agent is working in, and without that it shows up in git status and any acceptance criterion about a clean tree becomes unsatisfiable.

Verified

  • gofmt, go build ./..., go vet ./..., go test -race -count=1 ./... — all green, nothing regressed.
  • Real furrow binary, end to end: a workspace with an uncommitted edit, an untracked .env and a file written after attach all materialize byte-exact on a separate store, with a usable .git.
  • Real transport, end to end: furrow clone ssh://… through furrow-dial → TLS → furrowdfurrow __remote delivers the workspace; a wrong token is rejected.
  • Live node against a real control plane: furrow discovered via sibling-binary resolution, get_workspace_handle registered, and an unknown run answers available:false rather than erroring. (Measured before mirroring became opt-in; a default node now registers 31 reasoners and mounts get_workspace_handle only with SWE_FURROW_ENABLED on.)

Not yet exercised: a full LLM-backed build (needs a provider key in the node's environment) and the cloud deployment (its node is still not_configured).

Notes for review

  • Freshness is bounded by publish cadence — level boundaries and completion — not a background watcher. watch --no-daemon is deliberate: no orphan daemons inside a container. A periodic publish while a run is in flight is the obvious next increment.
  • The namespace on the wire is furrow's blinded name, so furrowd cannot compare it to the registry's; the token is what scopes a connection, and the namespace is charset-validated and passed through. Comparing them rejected every real clone.
  • Attaching is restricted to a workspace whose .git is a real directory. A git worktree's .git is a file pointing outside the tree, so mirroring one would produce a clone with no object database.

Review hardening (second pass)

A follow-up review pass added one commit:

  • Honest handles: a public ssh:// handle was minted from FURROW_PUBLIC_ADDR alone, even when furrowd never resolved, failed to bind, or exhausted its restart budget — a valid-looking address nothing listens on. The supervisor now exposes a cached process+TCP health signal and the manager falls back to the run's dir: handle (one warning logged) when it is down.
  • Durable stores: store/remotes roots defaulted under the workspace tree — ephemeral in cloud containers, so a restart deleted every published store and the TLS identity. When AGENTFIELD_HOME is set (cloud images set /data) the defaults now live under it; explicit env vars still win.
  • Sanitized paths: the remote-store dir was joined from the raw run ID while only the namespace was sanitized; ..-shaped IDs could land — and later be swept with RemoveAll — outside the root. The dir now uses the sanitized namespace, with ./.. survivors collapsing to the fallback name.
  • Manifest honesty: bin resolution order corrected (/usr/local/bin first, vendored sibling second), and the client-side vars ssh:// consumers actually need (FURROW_DIAL_TOKEN, FURROW_DIAL_INSECURE, FURROWD_TLS_CERT/KEY) are now declared.

go build ./..., go vet ./..., go test -race -count=1 ./... — all green.

Correctness and trust-boundary pass (third)

A deep review with empirical repros found four real defects. All four are fixed here, each with a regression test that was confirmed to fail against the previous code.

  • Empty run ID shared one mirror between builds. build() read the run ID straight off the execution context, where it can be empty, and sanitization turned "" into the namespace run. Two builds that hit that case landed on one registry row, so build B's Attach returned build A's workspace path, recovery key and token. Reproduced. Fixed on both sides: build() and execute() now use the same RunID-else-RootWorkflowID fallback planning.Scout already uses for scoped credentials (which also means the deferred ClearScopedCredentials finally clears the scope Scout wrote), and Manager.Attach refuses an empty run ID outright — it is the registry key and the remote directory name, so an empty one is a key two builds share, not a label one is missing.

  • SWE_FURROW_MAX_GB=0 wiped every live mirror, hourly. Three of the four readers already treated 0 as unlimited; Sweep gated its eviction loop on maxBytes >= 0 and so read the no-cap setting as a zero-byte cap — walking the store every tick and retiring mirrors until the registry was empty. The most destructive setting available was the one that asked for no limits. Non-positive now skips budget eviction entirely; age expiry is unaffected.

  • Budget eviction deleted mirrors of runs that were still building. retire() documents that it "re-checks staleness under the lock so a run that became active in the meantime is left alone", but the budget path called it with maxAge 0, which disabled that check on the one path where the victim is chosen by size rather than age. retire now takes an explicit eligibility predicate, so "no rule" cannot be expressed by accident, and the budget path requires the candidate's last publish to be both unchanged since it was measured and at least an hour old.

  • get_workspace_handle handed recovery keys to any caller with a run ID. See the reasoner section above — redacted by default now, opt-in via SWE_FURROW_EXPOSE_SECRETS.

Alongside them:

  • Opt-in, for real. The manifest shipped SWE_FURROW_ENABLED="1", the node built the manager unconditionally, and New() only bailed on the literal string "0" — so this PR's own claim that the default was off was wrong. Default is now 0, the parse accepts 1/true/yes/on as on and treats everything else (including a typo) as off, and get_workspace_handle is gated on mirroring actually being enabled, mirroring the if pro.Available() gate beside it.
  • furrowd no longer parses recovery keys. It decoded registry.json into furrow.Entry, whose Key field is the run's recovery key, on every AUTH line — before any authentication succeeded. It now decodes a narrow local struct with only the token, namespace and store dir, so the one network-facing process in this feature never holds a recovery key at all.
  • Sweeper deletion is bounded. entry.StoreDir came off disk and went straight to os.RemoveAll. A row naming anything outside the remotes root (or the root itself, or the empty string) now drops the row and deletes nothing.
  • furrowd's namespace validation rejects a leading -. The namespace is attacker-supplied text that becomes an argv element of furrow __remote <namespace>; - is in the permitted charset, so a leading one arrives looking like a flag. Refused rather than assuming furrow's parser honours --.
  • A furrowd restart loop is no longer silent. The child's stdout and stderr went nowhere, so a daemon that could not bind its port restarted every backoff interval and left one line after five failures naming an exit status, never the reason. Both streams now reach the node's log, prefixed.
  • Two doc comments corrected. Attacher claimed callers never need a nil check (a nil *Manager is fine; a nil interface panics, which is why orch.Deps nil-checks it), and Detach claimed to stop mirroring when it only reports whether the run is still known.

Scope trim. This branch also carried an unrelated git checkout -B worktree-reclaim fallback in internal/issue/gitops.go plus the LC_ALL=C pin it needs, and their test file. Nothing in the furrow work depends on internal/issue, so they are reverted to main here. They are worth landing — separately, and with their own argument about whether "git's stderr does not contain a branch named" is a sound signal for "this leftover branch is mine to force-reset", given issue branches carry commits that outlive their build.

gofmt -l . empty, go build ./..., go vet ./..., go test -race -count=1 ./... — all green against the SDK at CI's pinned ref with GOWORK=off. No Python files touched.

AbirAbbas and others added 5 commits August 5, 2026 15:25
Defines the one-way surface orchestration depends on: a Handle that travels in
reasoner results, a registry Entry mapping a control-plane run ID to the
workspace on disk, and an Attacher whose nil implementation no-ops. Availability
is discovered by a caller finding a handle, never by asking.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
furrowd serves one run's encrypted blob store over TLS by spawning
`furrow __remote` behind a token check, and furrow-dial stands in as
FURROW_SSH_COMMAND on the caller's side so a stock `furrow clone` reaches it
with no patches to furrow.

This is deliberately not sshd. The cloud image runs the control plane and its
agent nodes in one container as root, so an ssh key there would be whole-box
access to every run and every secret on the volume. A furrowd token unlocks a
single run's data root, and what crosses the wire is ciphertext either way.

The namespace is passed through to the child rather than checked against the
registry: furrow blinds it (keyed BLAKE3) before it ever leaves the client, so
the node cannot recompute it, and comparing the two rejected every real clone.
The token is what scopes the connection; the namespace only selects a directory
beneath the root it already pins, so it is charset-validated instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A build now attaches its clone to furrow, republishes at every DAG level
boundary and at completion, and returns a workspace_handle in its result. A
caller that started the run can use that handle to clone the workspace and watch
it change while the build is still going — including the uncommitted edits,
untracked files and dev state that a git push would never carry.

Availability is discovered, never configured. The handle is present when the
mirror works and absent when it does not, so there is no flag to set, nothing to
probe, and no new failure a user has to understand. Every unavailable path — no
binary, no .git, attach or publish failing, the feature switched off — degrades
to exactly today's behaviour. A nil Attacher disables all of it, which is the
default.

Two things the mocked tests could not have caught, both found by running the
real binary:

  - The capture policy has to be written as `exclude <subtree>` lines. Bare
    names are rejected, `watch` then fails, and the mirror is off for every
    build with only a debug line to show for it. The exact bytes are pinned in
    a unit test now, and integration_test.go drives the real binary end to end
    so the whole CLI contract is checked rather than assumed.

  - .furrowpolicy lands in the repository the agent is working in, so it joins
    .artifacts/ and .worktrees/ in .git/info/exclude. Otherwise it shows up in
    `git status` and any acceptance criterion about a clean tree is
    unsatisfiable.

The run registry closes a real gap: SWE-AF knew its workspaces only by a locally
generated build ID, with nothing tying them to the control plane's run ID. That
mapping is what makes a workspace addressable from outside at all, and the
hourly sweeper bounds what it costs to keep (72h / 20GB by default) — furrow
never prunes a remote on its own.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
furrowd existed but nothing started it, which left the remote path unreachable
in exactly the deployment it was built for: in the cloud the control plane and
its agent nodes share one container, so there is no separate service to run it.
The node supervises it the same way it already supervises the coding engine —
backoff, a give-up threshold, and a process-group kill so nothing outlives the
node.

It stays inert unless there is something to serve: the mirror has to be enabled,
FURROW_PUBLIC_ADDR has to name an address the daemon can advertise, and a
furrowd binary has to resolve. Any of those missing is the normal case on a
developer's machine, so it is a debug line rather than a warning.

Verified against a live node rather than only a fake: furrowd comes up on its
port, speaks TLS, refuses an unauthenticated peer, and goes away when the node
does.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AbirAbbas and others added 3 commits August 5, 2026 16:00
The manager held a single lock for the whole of Attach and Publish, including
the furrow invocations inside them. A node serves several builds at once, and
an initial capture of a large repository is slow, so one build attaching could
park every other build's publish behind it — in a feature whose entire purpose
is to let parallel agents be watched while they work.

The lock now guards the registry only, and per-run locks provide the ordering
that actually matters: two calls for the same run must not both pair it, but
different runs have no reason to wait on each other. Publish also re-checks the
run still exists before recording a timestamp, so a sweep that retires a run
mid-push cannot resurrect a row whose store is already gone.

The test parks two publishes inside a fake furrow until both have arrived, which
can only happen if they run concurrently — verified to fail against the previous
locking and pass against this one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The hourly sweep held the registry lock while deleting store directories and
walking the whole store to measure it — the same mistake the previous commit
fixed for attach and publish, and a worse one, because retiring a run whose
store is gigabytes can take a while and every attach and publish on the node
waits behind it.

Deletion and measurement now happen with no lock held. Each run is retired under
its own lock, so a publish already in flight finishes rather than pushing into a
directory being deleted, and staleness is re-checked there: a run that became
active between the scan and the retire is left alone. Files go first and the
registry row second, so a failed delete leaves a row that the next sweep retries
instead of orphaning a store nothing points at.

The size pass stops if the run it picked survives its re-check, since measuring
again would keep choosing the same victim.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e it

The handle advertised the directory that holds every run's store rather than
the one belonging to this run. Pairing with it finds no workspace — `sync
--pull` fails with "sync remote has no published HEAD" — so the local path,
which is the default on a developer's machine whenever no public address is
configured, could not actually be used. Over the network the address was
always right, because furrowd resolves the store from the token.

The integration test now builds every argument from the handle instead of
assembling paths of its own. A consumer only ever has the handle, so a test
that reaches around it can pass while the handle itself is unusable — which is
exactly what happened here. Verified to fail against the old value.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AbirAbbas

Copy link
Copy Markdown
Collaborator Author

Reviewer note on the transport's threat model, since the client skips certificate verification and that deserves an explicit answer rather than a comment in passing.

What crosses the furrowd connection: the per-run token, then furrow's framed protocol carrying encrypted objects. Object contents, object ids and the workspace name are all encrypted or blinded by furrow before they reach the transport.

What does not: the recovery key. It travels in the workspace_handle inside the reasoner result — over the control plane's own TLS — and is never sent to furrowd. furrowd never has it either; it only shuttles bytes to furrow __remote, which stores ciphertext it cannot read.

So an attacker who fully MITMs the furrowd connection gets a token and a stream of ciphertext. The token lets them pull that one run's objects, which stay unreadable without the recovery key they do not have. They cannot read the workspace, cannot forge a snapshot the real client would accept (objects are BLAKE3-verified against their ids on restore), and cannot reach any other run — the token pins one data root.

That is why the default self-signed certificate is tolerable and why FURROW_DIAL_INSECURE exists. It is not a claim that certificate verification is worthless: an operator who sets FURROWD_TLS_CERT/FURROWD_TLS_KEY to a real certificate gets verification by leaving that variable unset, and that is the better posture on a shared network. The default is chosen so the feature works with no setup, not because the stronger option was unavailable.

The part I would push back on in review is that the token is sent before the client has authenticated the server, so a MITM harvests tokens even though they cannot use them for much. If we want that closed, the options are pinning furrowd's certificate fingerprint in the handle (the handle is already a confidential channel, so it can carry one) or a challenge-response that never puts the token on the wire. Neither is in this PR; the fingerprint would be a small addition if we want it before this is used across untrusted networks.

The installer builds only entrypoint.build and ignores dependencies.system for Go nodes. Cloud images therefore ship neither furrow nor furrowd, causing workspace mirroring to silently disable. Vendor the Linux amd64 siblings beside swe-planner so the existing resolver can find them.
Cloud secret injection only passes keys declared by the package. Expose the furrow client, daemon, storage, retention, and public-address settings with their code defaults so operators can configure persistent cloud deployments.
A newly paired run has no remote HEAD until its first snapshot and push, leaving handles unclonable throughout early build work. Publish an attached snapshot through the existing non-fatal path while retaining the per-run lock, and cover immediate real-binary materialization.
The binary and store-dir defaults are resolved at runtime (vendored sibling, /usr/local/bin, workspace root); a literal /workspaces path in the manifest would mislead local installs and risks being saved verbatim from config UIs.
furrow-dial runs on the CALLER's machine, not the node's: it is what
FURROW_SSH_COMMAND points at when a caller clones an ssh:// handle. Vendoring
only furrow and furrowd left the client half of the feature unshippable, since
furrow has no release channel and callers would have to build it from source.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AbirAbbas and others added 11 commits August 6, 2026 15:18
Review fixes for the workspace-handle feature:

- A public ssh:// handle was minted from FURROW_PUBLIC_ADDR alone, even
  when furrowd never resolved, failed to bind, or burned its restart
  budget — a valid-looking address nothing listens on. The supervisor now
  exposes a cached process+TCP health signal and the manager falls back
  to the run's dir: handle (with one warning) when it is down.
- Store and remotes roots defaulted under the workspace tree, which is
  ephemeral in cloud containers — a restart deleted every published
  store and the TLS identity. When AGENTFIELD_HOME is set (cloud sets
  /data) the defaults now live under it; explicit env vars still win.
- The remote-store directory was joined from the raw run ID while only
  the namespace was sanitized; '..'-shaped IDs could land (and later be
  swept with RemoveAll) outside the root. The dir now uses the sanitized
  namespace, and '.'/'..' survivors collapse to the fallback name.
- The manifest described bin resolution in the wrong order and never
  mentioned FURROW_DIAL_TOKEN / FURROW_DIAL_INSECURE / FURROWD_TLS_*,
  which ssh:// consumers need today.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TestConcurrentBuildsOnSameRepo went red in CI: 'git worktree add -b' can
create its branch and then lose the repo lock race, and the retry then dies
on 'a branch named ... already exists' — the recovery path was the failure.
The branch name embeds a per-call build ID nothing else can own, so -B
(create-or-reset) turns the leftover branch into recovery instead of an
error. Regression test simulates the leftover-branch state directly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…volume

Two hazards for deployments that upgrade into default-on mirroring:

- Furrow subprocesses ran with no timeout while holding the run's mutex, so
  a hung binary hung the build — before planning, at every DAG level, and at
  build return. Every invocation now runs under a five-minute deadline
  (Options.CmdTimeout) and reports the timeout instead of waiting forever.

- SWE_FURROW_MAX_GB only measured the remotes root, but every subprocess
  also writes a client store (FURROW_DATA_DIR) that was never counted or
  swept — unbounded growth on the /data volume that also holds the control
  plane's databases. The budget now means what the manifest says: aggregate.
  Attach refuses new mirrors when over budget (builds proceed unmirrored),
  and the sweeper measures both roots, warning once when the client store
  alone stays over the cap.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The earlier -B change was broader than its justification: issue branches
with commits are deliverables that outlive their build, and build IDs are
only 32 random bits, so an unconditional create-or-reset could silently
move a delivered branch on a name collision. Now the first attempt uses -b,
a branch that exists before anything went transiently wrong is a hard
failure again, and only retries after a non-exists failure — the case where
our own dying attempt may have created the branch — escalate to -B.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…raw run ID

The manager sanitizes the run ID into the directory it creates and records
that path as Entry.StoreDir, but the daemon rebuilt its data root from the
raw ID — so any ID sanitization alters was served from a directory that
does not exist, and a traversal-shaped ID named a path outside the remotes
root entirely. Serve the recorded path, falling back to the sanitized
namespace for entries that predate StoreDir.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…istic

addWorktree decides between refusing a pre-existing branch and reclaiming
our own leftover by matching git's stderr phrasing; a translated message
would have degraded the refusal into a reset. Run git under LC_ALL=C so
the phrase is the phrase. The leftover-recovery test also stops depending
on goroutine timing: the transient failure is scripted through a seam and
asserts the -b then -B attempt sequence, while the recovery itself still
runs real git against the leftover state.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…utable

Found by installing the node the way a user does. `af` copies
go/bin/furrow-linux-amd64 into the package as rw-r--r--, so ResolveBin
rejected it and every install logged 'no runnable furrow binary found' —
the entire feature silently off on exactly the platform it ships for.
runnable() now chmods a regular file that lost its execute bit and uses
it, and still rejects the candidate when the repair fails (read-only
filesystem, foreign owner).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Handle.Ref is documented as 'the furrow ref this run publishes to', but
publishLocked pushed with a bare 'sync --push', which writes the default
HEAD. Verified live against a real build: pulling with the advertised ref
fails 'sync remote has no published ref <run_id>', while the same pull
without --ref succeeds. Pass the ref on push so the field is true and the
per-run isolation it promises actually holds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…or's path

The execute-bit repair belonged on our own files, not on every probe.
runnable() is a pure predicate again, so SWE_FURROW_BIN and
/usr/local/bin/furrow are never rewritten — an explicit override that is
not executable still fails loudly. Only the sibling lookup, which searches
the binaries this package vendored beside its own executable, repairs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Handle.Ref claimed to name 'the furrow ref this run publishes to', but the
push wrote the default HEAD, so a consumer honoring it got 'sync remote has
no published ref'. Publishing under the named ref instead — the obvious
repair — turns out to BREAK the pull the shipped agentfield-use skill
documents (`sync --pull --bootstrap`, which reads the default HEAD);
verified live, that command then fails outright.

The field earns nothing either way: every run already gets its own remote
directory, so no remote ever holds two runs to disambiguate. Drop it, and
leave a note that a future shared-remote design must change the publish, the
handle, and the skill's recipe together.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AbirAbbas and others added 11 commits August 7, 2026 11:43
…adata

`furrow watch` creates .furrow/ inside the target repository, but the
hygiene list covered only .furrowpolicy — so after any furrow-enabled build
the user's own `git status` shows a permanent '?? .furrow/', and a
`git add -A` would commit it. Live runs only looked clean because the
build's agent happened to write a .gitignore containing .furrow/.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The disk budget counted the client store, but only the per-run remotes can
be reclaimed here. furrow self-caps its store at a global budget that
defaults to 20GiB — exactly our own default — so once the store alone
approached the cap, Sweep would retire every remote, still measure over
budget, and every later Attach would refuse forever: the mirror switching
itself off permanently with one log line and no way back.

Set furrow's own budget to half the allowance at startup. The total stays
inside SWE_FURROW_MAX_GB and the half we can actually free is always the
half that can grow.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cancellation closed the listener and then waited on every handler. A client
that authenticates and goes silent leaves its handler blocked copying input
forever, so SIGTERM never returns — the daemon hangs until killed. This
also made the suite fail under parallel load ('server did not shut down').
Track live connections and close them on cancel; a connection accepted in
the shutdown race is closed rather than served. The regression test fails
without the fix and the full race suite now passes repeatedly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A build's workspace mirror is filed under the control-plane run ID, but
build() read that ID straight off the execution context, where it can be
empty — the same context planning.Scout already handles by falling back
to the root workflow ID. Two builds that hit the empty case both landed
on the registry row for the sanitized namespace "run", so the second
Attach returned the FIRST build's RepoPath, recovery key and transport
token. Reproduced against the manager: two attaches with an empty ID
came back with one identical handle.

Fixed on both sides, because either one alone leaves the hole open:

  - build() and execute() now scope through scopeIDFromCtx, the same
    RunID-else-RootWorkflowID fallback planning.Scout uses. Attach and
    Publish therefore agree on the key, and the deferred
    ClearScopedCredentials now clears the scope Scout actually wrote.
  - Manager.Attach refuses an empty run ID outright. It is the registry
    key AND the remote directory name, so an empty one is not a missing
    label, it is a shared one — no fallback anywhere upstream can be
    trusted to have filled it in.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SWE_FURROW_MAX_GB=0 is the one setting an operator uses to say "do not
cap my disk", and three of the four places that read the budget already
agreed: configuredMaxBytes returns 0, alignStoreBudget returns early on
<= 0, and Attach only measures when the budget is > 0.

The sweeper did not. node.go turns the env var into 0 * GiB = 0 and
Sweep gated its eviction loop on `maxBytes >= 0`, so a zero budget was
read as a ZERO-BYTE cap: every hourly tick walked the store, found it
over "budget", and retired mirrors until the registry was empty —
including builds that were still running. The no-cap setting was the
most destructive one available.

Sweep now skips budget eviction entirely when maxBytes <= 0. Age expiry
is unaffected and still governed independently by maxAge.

TestAttachSanitizesRemoteStorePath swept with Sweep(0, 0) to trigger the
deletion it inspects; it now ages the entry past a real TTL instead, and
asserts the row is gone so the sentinel check cannot silently go vacuous
again.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
retire()'s doc promises it "re-checks staleness under the lock so a run
that became active in the meantime is left alone". That re-check was
`if maxAge > 0 && ...`, and the budget-eviction path called
retire(oldestID, 0) — so on the one path where the victim is chosen by
size rather than by age, the promise was disabled entirely. Whichever
run had published least recently lost its workspace, whether or not a
build was still writing to it.

retire now takes an explicit eligibility predicate instead of a
time.Duration, which makes "no rule" impossible to express by accident:

  - age expiry passes olderThan(maxAge), the previous behaviour;
  - budget eviction passes abandonedSince(observed), which requires the
    entry's last publish to be BOTH unchanged since the sweeper measured
    it and at least budgetGrace old (default one hour, matching the
    node's sweep cadence). A build publishes on attach, at every
    completed DAG level and at completion, so anything newer is live.

When the oldest mirror is not eligible the pass stops and logs: the
store stays over budget, Attach keeps refusing NEW mirrors, and an
operator can undo that by raising SWE_FURROW_MAX_GB. Deleting a running
build's mirror is not undoable, so it loses the tie.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The PR describing this feature said the default was off. It was not: the
manifest shipped SWE_FURROW_ENABLED="1", the node built the manager
unconditionally, and New() only bailed on the literal string "0" — so
every installed node with a vendored furrow binary started copying whole
build workspaces, untracked files and secrets included, into a second
on-disk store nobody asked for.

Three changes make the claim true:

  - the manifest defaults SWE_FURROW_ENABLED to "0", and its description
    says what mirroring actually copies;
  - New() gates on furrow.EnvTruthy, which reads 1/true/yes/on as ON and
    unset/0/false/no/off/anything-unrecognised as OFF. Same rule as
    pro.Enabled, and closed by default in both directions: "SWE_FURROW_
    ENABLED=disabled" used to enable it, and a bare node with no manifest
    now stays off instead of on;
  - get_workspace_handle is registered only when furrow is actually
    mirroring, mirroring the `if pro.Available()` gate beside it. It is
    an entrypoint-tagged reasoner whose whole job is handing out access
    to a live mirror; advertising it on a node that never makes one only
    invites a router to send traffic at a permanent {"available": false}.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ault

get_workspace_handle performs no authorization of any kind: it takes a
run_id and answers. It was returning the handle verbatim, which means it
handed every caller that could reach the node two secrets:

  - Key, the run's furrow recovery key, which decrypts that workspace —
    including the untracked files and credentials git never sees; and
  - Token, which authenticates to furrowd. furrowd serves the run's
    remote read-write, so a harvested token is not just a read.

The reasoner now withholds both by default and says so with
"secrets_redacted": true, returning the remote, namespace, version and
on-node repo path — everything a caller sharing the filesystem needs,
and enough for a human to see that a mirror exists. An operator running
a single-tenant cluster where every caller is already trusted with the
workspace contents opts back in with SWE_FURROW_EXPOSE_SECRETS=1.

The trust boundary is spelled out in the reasoner's own description (so
a routing model sees it), in the manifest entry, and above the renderer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lookupEntry decoded registry.json into map[string]furrow.Entry, and
Entry.Key is the run's furrow RECOVERY key — the secret that decrypts
that workspace. It runs on every AUTH line, before any authentication
has succeeded, so an unauthenticated stranger's connection was enough to
make the network-facing daemon page every recovery key on the node into
its address space.

furrowd now decodes into a local registryRow carrying only the three
fields it uses (namespace, token, store_dir). The key is not a field, so
it is never parsed, never resident, and not there to be dumped by a
crash, a core file or a read primitive in this process.

The manager still writes the full Entry; only the reader narrows. The
test pins both halves: registryRow must declare no "key"-tagged field,
furrow.Entry must still write one (so the assertion stays meaningful),
and a registry containing a real key must still authenticate and resolve
to the recorded store directory.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nd what a restart loop hides

Four small edges, none of them a live exploit on their own:

- retire() handed entry.StoreDir straight to os.RemoveAll. That path is
  read back out of a JSON file the node rewrites but does not own
  exclusively, and Attach only ever writes remotesRoot/<sanitized
  namespace>. A row naming anything else — a sibling directory, the
  remotes root itself, the empty string (which resolves to the process's
  working directory) — now drops the row and deletes nothing. `within`
  was already in the file for exactly this shape of check.

- furrowd's validNamespace permits '-' anywhere, and the namespace
  becomes an argv element of `furrow __remote <namespace>`. A LEADING
  '-' therefore reaches furrow looking like a flag. Rather than assume
  furrow's parser honours a "--" separator, refuse the shape: no
  namespace the manager derives from a run ID starts with '-'.

- the supervised furrowd's stdout and stderr went to /dev/null. A daemon
  that could not bind its port restarted every backoff interval in
  silence, and the only record was one line after five failures naming
  an exit status — never the reason, which was always on the child's
  stderr. Both streams now reach the node's log, prefixed and
  line-buffered, with the trailing partial line flushed.

- two doc comments described code that does not exist. Attacher claimed
  "callers never need a nil check" — a nil *Manager is fine, but callers
  hold the INTERFACE, and calling through a nil interface panics, which
  is why orch.Deps nil-checks the field. Detach claimed to stop
  mirroring; it only reports whether the run is still known.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This PR is about reaching a run's live workspace. Three changes in it
are not: issue.addWorktree's `-B` worktree-reclaim fallback, the
LC_ALL=C pin runGit needs to make that fallback's stderr matching
reliable, and the test file covering them. Nothing in the furrow work
depends on internal/issue — the packages do not reference each other —
so they come back out and the PR stays one thing.

They are worth landing on their own, and worth reviewing on their own,
because the fallback is data-loss-shaped: it decides "this leftover
branch is mine to reset" from the ABSENCE of the substring "a branch
named" in git's stderr, so any first-attempt failure whose message is
phrased differently promotes the retry to `git checkout -B`, which
force-moves an existing branch. Issue branches carry commits that
outlive their build. That reasoning deserves its own PR and its own
argument about whether stderr phrasing is a sound signal, not a slot in
a feature branch about workspace mirrors.

Reverted to origin/main: go/internal/issue/gitops.go,
go/internal/issue/exec.go; removed go/internal/issue/gitops_test.go,
which only exists to test them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AbirAbbas

Copy link
Copy Markdown
Collaborator Author

Two corrections to my threat-model comment above. Both were wrong in the direction of making the transport look safer than it was.

1. "furrowd never has it either" — the recovery key — was false. lookupEntry decoded registry.json into furrow.Entry, and Entry.Key is the run's recovery key. That decode ran on every AUTH line, before any authentication succeeded, so an unauthenticated connection was enough to make the network-facing daemon load every recovery key on the node into its address space. The key was never sent to furrowd over the wire, which is what I was thinking of — but it was already sitting on the same disk furrowd reads, and furrowd was reading all of it.

Fixed in this push: furrowd now decodes a narrow local struct carrying only the token, namespace and store dir. The key is not a field there, so it is never parsed and never resident.

2. "The token lets them pull that one run's objects" understated it. The token authenticates to furrow __remote, which is the remote server side — push as well as pull, and object deletion. So a harvested token buys write access to that run's remote, not just a read of it.

The confidentiality half of the original statement stands: the objects are ciphertext, the attacker still has no recovery key, so they cannot read the workspace. The integrity and availability half does not. A token holder can push objects into the run's store or delete what is there. Restore-side BLAKE3 verification means they cannot make the real client accept forged content — a tampered object fails its id check — but they can corrupt or empty the store, which turns a clone into a failure rather than a lie. Availability, not confidentiality, is the exposure.

Neither changes the conclusion about certificate pinning: the token is still sent before the client has authenticated the server, so a MITM still harvests it. That is now worth a bit more to the attacker than I said, which strengthens the case for putting furrowd's certificate fingerprint in the handle before this is used across an untrusted network. Still not in this PR.

Separately, and related to who can obtain a token in the first place: get_workspace_handle was returning both the recovery key and the token to any caller that could reach the node and name a run, with no authorization anywhere on that path. Both are now redacted by default ("secrets_redacted": true), with SWE_FURROW_EXPOSE_SECRETS=1 as the single-tenant opt-in.

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