fix(ui): confine agent/UI media paths to the workspace - #32
Merged
Conversation
The chat agent and the localhost `clip ui` routes dispatch model/HTTP-
supplied file paths straight into FFmpeg — via every tool in the registry
(POST /api/tools/:name + the agent's tool calls) and via the add_media
verb (POST /api/timeline/verbs). resolveInput returns absolute paths as-is,
so any of these untrusted surfaces could read an arbitrary file off disk:
POST /api/tools/render {"input":"/etc/passwd"} re-encodes it into a
workspace file the UI then serves back — an exfiltration channel. Violates
AGENTS.md non-negotiable #3.
Add resolveInWorkspace (relative resolves against the workspace; an absolute
path must already sit inside it; ..-traversal and out-of-tree absolutes throw
WorkspaceBoundaryError) and route EVERY path-bearing registry tool plus the
add_media verb through it — not just ingest. Both server routes map the error
to HTTP 403. The trusted CLI keeps resolveInput (a user-typed path is consent)
and never dispatches through the registry, so editing files anywhere on disk
still works.
Containment is by resolved-path prefix and does not yet follow symlinks
(documented residual: an in-workspace symlink can still point out — hardening
via realpath is a follow-up). The boundary message is audience-neutral so it
reads sensibly both as agent tool-result data and in the 403 body.
|
🎉 This PR is included in version 0.2.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Confines media file paths supplied by untrusted surfaces — the chat agent and the localhost
clip uiHTTP routes — to the workspace, enforcing AGENTS.md non-negotiable #3 ("File paths from MCP/agent tools are resolved against a workspace directory. No reads/writes outside it without explicit user consent"). The trusted CLI stays unconfined: a user typing a path is consent.Why (the hole)
resolveInputreturns absolute paths as-is, and the agent/UI feed model- or HTTP-supplied paths straight into FFmpeg through:POST /api/tools/:name(unauthenticated localhost) and the agent's tool calls, andadd_mediaverb —POST /api/timeline/verbs.So any of these could read an arbitrary file off disk. Concretely,
POST /api/tools/render {"input":"/etc/passwd"}re-encodes the file into a workspace output the UI then serves back viaGET /api/output/:opId— an arbitrary-file-read + exfiltration channel.Fix
resolveInWorkspace(path)(new, insrc/workspace.ts): relative paths resolve against the workspace; an absolute path must already sit inside it;..-traversal and out-of-tree absolutes throwWorkspaceBoundaryError.ingest.tool-registry.tsnow declares each tool's source-path field(s) (input/inputs[]/inputA,inputB/foreground,background/audio/overlay/path) and a single confining wrapper appliesresolveInWorkspaceat the dispatch boundary.add_mediaverb (makeVerbContext().ingest) is confined the same way.WorkspaceBoundaryError→ HTTP 403.resolveInputand never dispatches through the registry, so editing files anywhere on disk still works.Adversarial review
A 2-lens review (containment-bypass + gating-completeness) → verify-each was run before pushing. It confirmed the prefix-containment check is correct (no
startsWith-prefix bug; rejects traversal, out-of-tree absolutes, the workspace dir itself, cross-drive) and caught that an earlier draft gated onlyingestwhile the other ~16 registry tools stayed open — hence the comprehensive confinement here. The agent-unfriendly error message (advising a CLI the agent can't run) was also flagged and reworded to be audience-neutral.Residual (documented, not closed here)
Containment is by resolved-path prefix and does not follow symlinks — a symlink already present inside the workspace can still point out. Disclosed in the
resolveInWorkspaceJSDoc; hardening viarealpathis a follow-up.Tests
tests/workspace.test.ts: accepts in-workspace absolute/relative paths and a file literally named..foo; rejects..-traversal, out-of-tree absolutes (/etc/passwd), and the workspace dir itself; the CLI resolver stays unconfined. Registry coverage now assertsrender/trimreject out-of-workspace input, multi-path tools confine every field (add_audio), and array inputs are confined element-wise (concat).Gate green:
pnpm type-check,pnpm test(533 pass),pnpm lint. CLI smoke confirmedclip timeline add-media <out-of-workspace file>still succeeds.🤖 Generated with Claude Code