Skip to content

The workspace refuses a read through a symlink and allows a write through one #73

Description

@Hotragn

agent-computer/src/workspace.ts has three layers of confinement, and the module comment describes
the third as the one people miss: resolve what is on disk, because "a symlink makes the text a lie".
It does that for a read. For a write it resolves the directory and not the name, and writeFile
follows a link at the last component exactly as readFile does.

Where it is

resolvePath, one line (agent-computer/src/workspace.ts:120):

const anchor = forWrite ? dirname(target) : target;

A read resolves target, so a link at notes.txt is followed and the destination is checked. A
write resolves dirname(target), which proves where a new file would be created and says nothing
about a name that is already there. Then (:137):

return forWrite ? target : realAnchor;

The lexical target is handed back and written to. The comment above it is accurate about what it
proves and does not claim the last component:

For a write, return the full lexical target. It is already proven contained lexically, and the
deepest existing directory is proven contained after symlinks, so mkdir -p can only create the
rest inside the workspace.

So notes.txt/outside/secret.txt passes every layer: no .., not absolute, and its parent is
the workspace root, which resolves to itself.

Confirmed by running it

Against upstream/main unmodified, with a link at notes.txt pointing outside the workspace:

await expect(workspace().write("notes.txt", "owned")).rejects.toThrow(WorkspacePathError)

error: expect(received).rejects.toThrow(expected)
Expected promise that rejects
Received promise that resolved: Promise { <resolved> }

The write resolved. The file outside was overwritten.

Five shapes reproduce, all of them refused for a read today:

Case On main
link at the last component, pointing outside write succeeds
same, with { append: true } write succeeds
dangling link pointing at a name that does not exist yet write succeeds, creates the file outside
chain of two links ending outside write succeeds
cycle of two links raw ELOOP out of writeFile rather than a refusal

The dangling case is the one worth separating out. realpath throws on a link whose destination
does not exist, so a check built on realpath treats it as "nothing there" and takes the
forWrite branch at :124-131, while writeFile creates the destination regardless. It escapes
through the failure path rather than the success one.

What the existing tests cover, and the gap between them

agent-computer/tests/workspace.test.ts already has both neighbours of this:

  • refuses to read THROUGH a symlink that points outside — the read side.
  • refuses to write THROUGH a symlinked directory that points outside — a link standing in for a
    directory, which dirname resolution does catch.

A link standing in for the file is the case between them, and it is the one that is not there.
That is why this survived: the write side looks covered.

Why this is worth fixing even though a shell can write outside anyway

It is not a fresh escape for a Bot that already holds run_command; that Bot can write outside
directly, and #66 is about what that shell can reach. The link also has to get there first, which
takes a shell, or an archive unpacked by one.

What it is, is a hole in what the gateway can still see. The gateway decides and writes the audit row
in a different process, before agent-computer resolves anything — the policy context and the row
are both built from the path as the Bot asked for it (server/src/computer/gateway.ts:352, calling
describeFile at :700-714 on the requested path):

...(filePath ? { file: describeFile(filePath) } : {}),

So for a write aimed through a link:

  • a CEL rule written against file.path, file.name or file.extension is evaluated against
    notes.txt and never sees the file that gets written. A deny rule for credentials/ or *.env
    does not fire.
  • the refusal sentence a person would read names the claimed path
    (server/src/computer/policy.ts:306).
  • the audit row names a file in the workspace that nothing touched.

A deployment that denies run_command and allows write_file is relying on exactly that, and so is
anybody reading the trail afterwards. The same argument #68 landed on for bash -lc: a permissive
shell is a decision a deployment can make, and a trail that describes something other than what ran
is not.

Proposed fix

Resolve the last component too, and confine rather than forbid — a link pointing back inside the
workspace keeps working, the same as on the read side.

Walked with lstat/readlink rather than realpath, because of the dangling case above: realpath
throws where the write would still land, so the walk has to survive a link whose destination does not
exist yet. Bounded hops so a cycle is refused with a WorkspacePathError instead of surfacing an
ELOOP from the write.

A branch is ready with this and six tests — the five cases in the table plus one asserting a link
that points back inside still works. On upstream/main five of them fail and that sixth passes; with
the change all thirty-two in the file pass. PR follows immediately, so nobody need pick this up.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions