Skip to content

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

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

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