Skip to content

ContainerBackend accepts ignore patterns that name the mount point, which computerd then refuses at startup #195

Open

Description

@pucedoteth

Describe the bug

ContainerBackend checks ignore patterns in its constructor so that "a typo fails before a container starts rather than as a daemon that exits during startup" (ignore-assertion.ts). The check skips one step that computerd's resolveMountIgnore takes: computerd removes the mount point first (/workspace/dist becomes /dist), then runs the whole-mount check. The client never removes it. So a pattern that names the mount point passes the constructor, and then computerd refuses it and exits:

ignore pattern checkIgnorePatterns computerd resolveMountIgnore(…, "/workspace")
/* rejects rejects
/workspace accepts rejects: would make the whole mount local-only
/workspace/* accepts rejects: same
/workspace/** accepts rejects: same
/workspace/**/* accepts rejects: same

The two test suites, which the comment says "pin the same cases", have drifted too. computerd's whole-mount test includes "/workspace" and "/workspace/**", and the client's test does not.

Expected behavior

new ContainerBackend({ ..., ignore: ["/workspace/**"] }) throws the same "would make the whole mount local-only" error that ignore: ["/**"] does, without starting a container.

Steps to reproduce

On main (2f76387), with the two modules side by side:

import { checkIgnorePatterns } from "./packages/computer/src/backends/container/ignore-assertion.ts";
import { resolveMountIgnore } from "./packages/computerd/src/fuse/ignore.ts";

checkIgnorePatterns(["/workspace/**"]);              // no error
resolveMountIgnore(["/workspace/**"], "/workspace"); // MountIgnorePathError: ... would make the whole mount local-only

Proposed fix

Give checkIgnorePatterns the mount point and remove it the way computerd does, before the segment checks. The backend passes containerEnv?.MOUNT_POINT ?? "/workspace", which is the same value it puts in the container's env.

-export function checkIgnorePatterns(patterns: readonly string[]): void {
+export function checkIgnorePatterns(patterns: readonly string[], mountPoint = "/workspace"): void {
+  const root = mountPoint.replace(/\/+$/, "");
   for (const raw of patterns) {
 ...
-    const body = pattern.startsWith("!") ? pattern.slice(1) : pattern;
+    let body = pattern.startsWith("!") ? pattern.slice(1) : pattern;
 ...
     if (body.includes(",")) {
       fail("contains a comma, which separates patterns in MOUNT_IGNORE.");
     }
+    // computerd drops the mount point first: "/workspace/dist" is "/dist".
+    if (root !== "" && (body === root || body.startsWith(`${root}/`))) {
+      body = body.slice(root.length) || "/";
+    }
-    if (options.ignore !== undefined) checkIgnorePatterns(options.ignore);
+    if (options.ignore !== undefined) {
+      checkIgnorePatterns(options.ignore, options.containerEnv?.MOUNT_POINT ?? "/workspace");
+    }

Tests: "/workspace" and "/workspace/**" are added to the client's whole-mount list, so it matches computerd's. A new case covers /workspace/*, /workspace/**/*, a custom MOUNT_POINT, and confirms /workspace/dist is still accepted. On main these fail (2 failed, 24 passed). With the change, ignore-assertion.test.ts passes 26/26. I can share the full patch if useful.

Environment

@cloudflare/computer 0.4.0, main at 2f76387. Found by reading the code, then confirmed by calling both functions under Node 22. I have not deployed a container to see the startup failure end to end.

Activity

  1. VG31OP commented on Oct 17, 2026

    @VG31OP

    Hi @aron-cf @pucedoteth,

    Per the contribution guidelines in CONTRIBUTING.md, I have implemented and validated the complete fix for this issue with full test coverage on my fork:
    👉 Commit: 24b3298 on branch fix/container-ignore-mountpoint.

    Summary of Changes:

    1. Strip Mount Point Prefix in checkIgnorePatterns:
      • Updated checkIgnorePatterns(patterns: readonly string[], mountPoint: string = "/workspace") in packages/computer/src/backends/container/ignore-assertion.ts to normalize the mount point root and strip the prefix from pattern bodies before whole-mount segment evaluation, exactly mirroring computerd's resolveMountIgnore.
    2. Pass Configured Mount Point in ContainerBackend:
      • Passed options.containerEnv?.MOUNT_POINT ?? "/workspace" to checkIgnorePatterns in ContainerBackend.
    3. Comprehensive Regression Tests:
      • Extended packages/computer/src/backends/container/ignore-assertion.test.ts to cover whole-mount and wildcard patterns with the default /workspace mount point (such as "/workspace", "/workspace/*", "/workspace/**", and "!/workspace/**"), custom mount points (e.g. /app), and verifying subpath ignores (/workspace/dist) remain accepted.
    4. Changeset:
      • Added .changeset/container-ignore-mountpoint.md documenting the patch.

    Validation:

    • vitest run src/backends/container/ignore-assertion.test.ts: 26/26 tests passed.
    • All container backend tests: 9 test files, 87/87 tests passed.
    • oxlint and oxfmt --check: 0 errors, 0 warnings.

    The patch is ready to be pulled or cherry-picked whenever convenient!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions