feat(project): a filesystem confined to the project root, for a project from elsewhere by raymondk · Pull Request #866 · dfinity/icp-cli

raymondk · 2026-11-04T00:32:45Z

This is the library that a user (like the mcp server) would use to load an untrusted project.

Loading a project holds nothing: the loader joins each path a manifest names to its base and reads it through the host filesystem. Right for the user's own tree, wrong for one someone else handed over, where a manifest can name ../../etc/hostname as an argument file and have it read into the canister's arguments at load, before any caller could check.

files::Confined is the host filesystem held to one directory by the spelling of each path. A relative path is joined to the root, . dropped, .. applied, and a path that comes to rest outside the root — by climbing, or by being absolute elsewhere — is refused with OutsideRoot. The predicates answer false for such a path, scratch directories are made under the root, and resolve hands out the real path to a caller that needs one. The root is matched as the caller spells it and never canonicalized, which is what keeps it working on macOS (/var is /private/var) and Windows.

The check is lexical; symlinks are the business of whoever puts the tree on disk. A consumer of an untrusted bundle refuses any archive entry that is not a regular file or directory on the way in (#863), after which the tree has no links and Confined is complete; #868 makes the bundler write such archives. A local recipe template is the one file the loader does not read through this seam: the Resolve implementation fetches it, and a consumer must confine or refuse that itself.

The loader resolves every path through the filesystem it is given, so confining that one seam covers all of them, canister directories and dependency projects included. The tests in project.rs load one manifest through both filesystems to show the difference. The CLI keeps loading through HostFileSystem, so there is no changelog entry.

Consumer: dfinity/imcp2#230 pins this branch and replaces its own index-backed filesystem with Confined.

Ref: #865

🤖 Generated with Claude Code

Copilot

🟡 Changes recommended

Critical confinement gaps allow public resolution, dangling symlinks, and concurrent symlink replacement to bypass the project root and access or create files outside it.

4 open issues

Public path resolution skips symlink containment checks · New
Path-based checks allow symlink race escapes · New
Dangling symlinks can write outside the root · New
Local recipe reads bypass filesystem confinement · New

What changed in this PR

Adds an opt-in Confined filesystem to icp-project for loading externally supplied projects within a designated root, while leaving the CLI’s HostFileSystem behavior unchanged.

Changes:

Adds path containment checks, confinement errors, and root-local scratch directories.
Adds filesystem tests for traversal, symlinks, and scratch-file operations.
Adds loader tests comparing host and confined access to outside argument files and canister directories.

File	Description
`crates/icp-project/src/project.rs`	Adds integration tests comparing host and confined project loading.
`crates/icp-project/src/files.rs`	Implements `Confined`, its public API and errors, and filesystem tests.

_{Review effort: Balanced}

The index-backed filesystem went: the confinement it provided is now the library's own. dfinity/icp-cli#866 (issue #865) adds `files::Confined`, the host filesystem held to one directory: every path is resolved against the root before the disk sees it, and one that comes to rest outside — by climbing, by being absolute elsewhere, or through a symlink inside pointing out — is refused. The loader resolves every path a manifest names through the filesystem it is given, so that one seam holds all of them, canister directories included, which is also why the spelling check on canister directories went: a directory outside the root is not found, and the project does not load. What stays here is what the library cannot know about an archive: the bounds on its size, its decompressed size, its files and directories and their paths, the refusal of symlinks, device nodes and `.icp`, the metered view the loader reads through, and the checks on the bundle's shape before any phase runs. The four icp-cli crates are pinned to that branch's head until it is on main. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

A symlink inside a plugin's `files:` or `dirs:` directory was recorded as a link entry with its target unchecked, so a link to a file outside the project reached that file on whatever machine unpacked the bundle. A `files:` entry that was itself a symlink to a file outside the project was read straight into the bundle: it was validated lexically before the build, when it need not have existed, and the read followed the link. The archive walk is now the bundler's own rather than tar's. Every entry met is canonicalized and must land under the canonical project directory: one that does is appended as what it points at, so a project may link to its own files freely, and one that does not fails the bundle, naming the link and where it leads. A dangling link, a link back to a directory being copied, and anything that is neither a file nor a directory fail the same way. A `files:` entry is held to the project again once the build has run and it exists. The walk appends entries in sorted order, which makes the archive reproducible for a given tree where tar's own walk left the order to the filesystem. This is the bundling side of the rule that `files::Confined` (#866) relies on: a bundle holds no symlinks, a consumer refuses any archive entry that is not a regular file or directory (#863), and so the tree a confined loader reads has nothing in it that leads elsewhere. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…ct from elsewhere `icp project bundle` holds every path a manifest names to the project it is bundling, and refuses one that resolves outside. Loading a project holds nothing: the loader joins each path to its base and reads it through the host filesystem, which is right for the user's own tree and wrong for a project someone else handed over, a bundle an agent passes to a deploy service, say. A manifest in such a bundle could name `../../etc/hostname` as an argument file and have it read into the canister's arguments at load, before any caller could check. `files::Confined` is the host filesystem held to one directory. Every path is resolved against the root before the disk sees it: a relative path is joined, `.` dropped, `..` applied, and a path that comes to rest outside the root, by climbing, by being absolute elsewhere, or through a symlink inside the root that points out, is refused with `OutsideRoot`. The predicates answer false for such a path, scratch directories are made under the root, and `resolve` hands out the real path for a caller that needs one for something that does not read through the trait. The loader resolves every path a manifest names through the filesystem it is given, so confining that one seam holds all of them, canister directories and dependency projects included, without the loader enumerating its fields; the tests load one manifest through both filesystems to show the difference. Nothing changes for the CLI, which keeps loading through `HostFileSystem`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The symlink walk was the wrong layer. A symlink is the business of whoever puts a tree on disk: bundling writes only regular files and directories, and a consumer unpacking an archive it does not trust refuses any entry that is not, so the tree Confined reads has no links to follow. The user's own tree may link wherever it likes, and the host follows it. Confined now resolves every path lexically against the root and does nothing else. Canonicalizing the root in `new` is also what failed CI on macOS and Windows: the root was stored as `/private/var/...` or `runneradmin`, while every absolute path the loader passed was spelled as the caller got it (`/var/...`, `RUNNER~1`), and so refused. The root is now kept as spelled, and a unix test reproduces the failure through a symlinked alias of the project directory. The doc claim that one seam holds every loader read is narrowed: a local recipe template is read by the `Resolve` implementation, which a consumer holding an untrusted tree must confine or refuse itself. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

A symlink inside a plugin's `files:` or `dirs:` directory was recorded as a link entry with its target unchecked, so a link to a file outside the project reached that file on whatever machine unpacked the bundle. A `files:` entry that was itself a symlink to a file outside the project was read straight into the bundle: it was validated lexically before the build, when it need not have existed, and the read followed the link. The archive walk is now the bundler's own rather than tar's. Every entry met is canonicalized and must land under the canonical project directory: one that does is appended as what it points at, so a project may link to its own files freely, and one that does not fails the bundle, naming the link and where it leads. A dangling link, a link back to a directory being copied, and anything that is neither a file nor a directory fail the same way. A `files:` entry is held to the project again once the build has run and it exists. The walk appends entries in sorted order, which makes the archive reproducible for a given tree where tar's own walk left the order to the filesystem. This is the bundling side of the rule that `files::Confined` (#866) relies on: a bundle holds no symlinks, a consumer refuses any archive entry that is not a regular file or directory (#863), and so the tree a confined loader reads has nothing in it that leads elsewhere. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

lwshang · 2026-11-05T12:48:57Z

+            .await
+    }
+
+    async fn exists(&self, path: &Path) -> bool {


These answer false for a path outside the root. So a canister directory like canisters: [../elsewhere] fails as "not found" rather than "outside the root", and the test in project.rs admits this ("the directory is not there"). For whoever is debugging a rejected bundle, an error that says the path is outside the root is more useful. Is the canister-directory lookup worth routing through resolve first, so the OutsideRoot error comes through?

lwshang · 2026-11-05T12:48:57Z

+/// `RUNNER~1`) is understood as long as it keeps to the same spelling.
+#[cfg(feature = "host")]
+#[derive(Debug, Clone)]
+pub struct Confined {


It's worth saying in this doc that Confined confines what the loader reads, and nothing else. A script build step in an untrusted manifest still runs arbitrary commands through Build, so a consumer like imcp2 also has to refuse script steps or run them sandboxed. One sentence here would stop someone treating Confined as the whole sandbox.

Copilot AI balanced review requested due to automatic review settings November 4, 2026 00:32

Copilot started reviewing on behalf of raymondk November 4, 2026 00:33

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

Comment thread crates/icp-project/src/files.rs Resolved

Comment thread crates/icp-project/src/files.rs Resolved

Comment thread crates/icp-project/src/files.rs Resolved

Comment thread crates/icp-project/src/files.rs Resolved

raymondk added this pull request to stack #869 November 4, 2026 19:54

raymondk marked this pull request as ready for review November 5, 2026 01:09

raymondk requested a review from a team as a code owner November 5, 2026 01:09

raymondk and others added 2 commits November 5, 2026 07:46

raymondk force-pushed the rk/confined-project-load branch from c2b8b36 to 6f5b535 Compare November 5, 2026 12:46

lwshang reviewed Nov 5, 2026

View reviewed changes

lwshang approved these changes Nov 5, 2026

View reviewed changes

raymondk mentioned this pull request Nov 5, 2026

feat(project): an untrusted project is held to the shape a bundle has #871

Draft

Navigation Menu

Repository navigation

feat(project): a filesystem confined to the project root, for a project from elsewhere - #866

feat(project): a filesystem confined to the project root, for a project from elsewhere#866
raymondk wants to merge 2 commits into
mainfrom
rk/confined-project-load

raymondk commented Nov 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lwshang Nov 5, 2026

Uh oh!

lwshang Nov 5, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

raymondk commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lwshang Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

lwshang Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

raymondk commented Nov 4, 2026 •

edited

Loading