linux: a wrap that starts over reads the disk again

src/sandbox/read-deny-glob.ts

@@ -1,7 +1,6 @@
 import { logForDebugging } from '../utils/debug.js'
 import {
   type GlobWalkListings,
-  type GlobWalks,
   isAtOrUnder,
   normalizePathForSandbox,
   pathSpellings,
@@ -92,23 +91,17 @@ export function expandReadDenyGlobLinux(
   )
 }
 
-/**
- * {@link expandReadDenyGlobLinux}, in the walk's steps. With `walks`, a
- * pattern walked before is not walked again; what follows the walk is done
- * anew each time, since `reExposedPaths` may differ.
- */
+/** {@link expandReadDenyGlobLinux}, in the walk's steps. */
 export function* expandReadDenyGlobLinuxSteps(
   globPattern: string,
   reExposedPaths: readonly string[],
   unlistableDirs?: Set<string>,
   listings?: GlobWalkListings,
-  walks?: GlobWalks,
 ): Steps<string[]> {
   const walk = yield* walkGlobPatternSteps(globPattern, {
     withDirectoryForm: true,
     followSymlinkedDirectories: true,
     listings,
-    walks,
   })
   // Where a path the walk reported really lives: the denyRead loop mounts an
   // entry there, whatever spelling named it.

src/sandbox/sandbox-manager.ts

@@ -86,7 +86,6 @@ import {
   removeTrailingGlobSuffix,
   walkGlobPatternSteps,
   type GlobWalkListings,
-  type GlobWalks,
   type Steps,
   finish,
   finishInTurns,
@@ -1308,11 +1307,8 @@ function stripWriteGlobs(paths: readonly string[]): string[] {
     })
 }
 
-function* expandAllowReadGlob(
-  pattern: string,
-  walks?: GlobWalks,
-): Steps<string[]> {
-  const expanded = (yield* walkGlobPatternSteps(pattern, { walks })).matches
+function* expandAllowReadGlob(pattern: string): Steps<string[]> {
+  const expanded = (yield* walkGlobPatternSteps(pattern)).matches
   logForDebugging(
     `[Sandbox] Expanded allowRead glob pattern "${pattern}" to ${expanded.length} paths on Linux`,
   )
@@ -1656,27 +1652,22 @@ export type WrapWithSandboxOptions = {
   commandText?: string
 }
 
-/** What a wrap has found on disk so far, which it keeps when it starts over. */
-type WrapFindings = {
-  listings: GlobWalkListings
-  walks: GlobWalks
-  /** How many times the wrap has started over. */
-  restarts: number
-}
-
 /** A wrap that has started over this many times walks without giving the
  *  thread back. */
 const RESTARTS_IN_TURNS = 2
 
+/** In place of what a walk finds: the configuration was replaced under it. */
+const REPLACED = Symbol('replaced')
+
 async function wrapWithSandbox(
   command: string,
   binShell?: string,
   customConfig?: Partial<SandboxRuntimeConfig>,
   abortSignal?: AbortSignal,
   options?: WrapWithSandboxOptions,
 ): Promise<string> {
-  return wrapWithSandboxKeeping(
-    { listings: new Map(), walks: new Map(), restarts: 0 },
+  return wrapWithSandboxAgain(
+    0,
     command,
     binShell,
     customConfig,
@@ -1685,8 +1676,9 @@ async function wrapWithSandbox(
   )
 }
 
-async function wrapWithSandboxKeeping(
-  found: WrapFindings,
+/** {@link wrapWithSandbox}, having started over `restarts` times. */
+async function wrapWithSandboxAgain(
+  restarts: number,
   command: string,
   binShell?: string,
   customConfig?: Partial<SandboxRuntimeConfig>,
@@ -1697,9 +1689,30 @@ async function wrapWithSandboxKeeping(
   const commandId = options?.commandId
   registerCommandText(command, options)
   const startedWith = config
-  const walked = <T>(steps: Steps<T>): T | Promise<T> => {
-    if (found.restarts < RESTARTS_IN_TURNS) {
-      return finishInTurns(steps, abortSignal)
+  const startOver = (): Promise<string> =>
+    wrapWithSandboxAgain(
+      restarts + 1,
+      command,
+      binShell,
+      customConfig,
+      abortSignal,
+      options,
+    )
+  /** `steps`, left as soon as the configuration is seen to be replaced:
+   *  what they would go on to find is for a wrap that starts over anyway. */
+  function* whileCurrent<T>(steps: Steps<T>): Steps<T | typeof REPLACED> {
+    for (;;) {
+      if (config !== startedWith) return REPLACED
+      const step = steps.next()
+      if (step.done) return step.value
+      yield
+    }
+  }
+  const walked = <T>(
+    steps: Steps<T>,
+  ): T | typeof REPLACED | Promise<T | typeof REPLACED> => {
+    if (restarts < RESTARTS_IN_TURNS) {
+      return finishInTurns(whileCurrent(steps), abortSignal)
     }
     abortSignal?.throwIfAborted()
     return finish(steps)
@@ -1768,9 +1781,10 @@ async function wrapWithSandboxKeeping(
         customConfig?.filesystem?.allowRead ??
           config?.filesystem.allowRead ??
           [],
-        pattern => expandAllowReadGlob(pattern, found.walks),
+        expandAllowReadGlob,
       ),
     )
+    if (expandedAllowRead === REPLACED) return startOver()
     // The TLS-termination CA cert and the trust bundle the env vars point at
     // (NODE_EXTRA_CA_CERTS etc.) must be readable by the child, even if their
     // paths fall under a user-configured denyRead.
@@ -1783,6 +1797,7 @@ async function wrapWithSandboxKeeping(
     }
     const reExposedPaths = [...expandedAllowRead, ...writeConfig.allowOnly]
     const unlistableDenyDirs = new Set<string>()
+    const listings: GlobWalkListings = new Map()
     const expandedDenyRead = await walked(
       resolveReadPathEntries(
         unionDenyReadPaths(
@@ -1796,12 +1811,12 @@ async function wrapWithSandboxKeeping(
             pattern,
             reExposedPaths,
             unlistableDenyDirs,
-            found.listings,
-            found.walks,
+            listings,
           ),
         credentialRestrictions.degradeToDenyPaths,
       ),
     )
+    if (expandedDenyRead === REPLACED) return startOver()
     readConfig = {
       denyOnly: expandedDenyRead,
       allowWithinDeny: expandedAllowRead,
@@ -1839,23 +1854,21 @@ async function wrapWithSandboxKeeping(
   // allowGitConfig of another is a policy nobody chose. So the wrap starts
   // over. Nothing from here to the platform's wrapper awaits.
   //
-  // INVARIANT: starting over costs only what is new, and ends. An embedder
-  // can replace the configuration many times during one walk, mostly with
-  // the same content. So the wrap keeps what it has listed and walked: an
-  // attempt with no pattern new to it takes no turn of the event loop, and
-  // after RESTARTS_IN_TURNS the new ones are walked without a turn as well.
-  // What is kept is as old as its walk, and misses a path made since, as one
-  // long walk misses a path made behind it.
-  if (config !== startedWith) {
-    return wrapWithSandboxKeeping(
-      { ...found, restarts: found.restarts + 1 },
-      command,
-      binShell,
-      customConfig,
-      abortSignal,
-      options,
-    )
-  }
+  // INVARIANT: what a wrap hands out was read from the disk after its
+  // configuration was installed. A host that writes a file and then installs
+  // a rule that denies it finds it denied by every wrap that goes by that
+  // rule. So a wrap that starts over keeps nothing of what it has listed or
+  // walked, the same rules included. A configuration installed after this
+  // check is the next wrap's.
+  //
+  // An embedder can replace the configuration many times during one walk.
+  // After RESTARTS_IN_TURNS the wrap walks without a turn of the event loop,
+  // so no timer or I/O callback replaces it meanwhile. That holds the thread
+  // for one walk, as every wrap did before the walk took turns. It is not a
+  // proof that the wrap ends: a promise continuation, or anything at all
+  // while the network is still being initialized, can replace it between the
+  // walk and this check, and the wrap then starts over once more.
+  if (config !== startedWith) return startOver()
 
   // Check custom config to allow pseudo-terminal (can be applied dynamically)
   const allowPty = customConfig?.allowPty ?? config?.allowPty

src/sandbox/sandbox-utils.ts

@@ -1141,19 +1141,12 @@ export interface ExpandGlobOptions {
  *  pattern, reads the same entries. */
 export type GlobWalkListings = Map<string, fs.Dirent[]>
 
-/** Finished walks, by their options and pattern. */
-export type GlobWalks = Map<string, GlobWalk>
-
 export type GlobWalkOptions = ExpandGlobOptions & {
   withDirectoryForm?: boolean
   followSymlinkedDirectories?: boolean
   /** Handed to every walk of one configuration, so that patterns with a
    *  base in common list each directory once between them. */
   listings?: GlobWalkListings
-  /** A walk found here is handed back as it is, and a finished one is put
-   *  here: for a caller that starts over and should walk no pattern twice.
-   *  What such a walk reports is as old as the walk. */
-  walks?: GlobWalks
 }
 
 /** Work that can be left between two steps and taken up again. */
@@ -1550,18 +1543,6 @@ export function* walkGlobPatternSteps(
   globPath: string,
   opts: GlobWalkOptions = {},
 ): Steps<GlobWalk> {
-  // The options that shape a walk, then its pattern.
-  const walkKey =
-    [
-      opts.withDirectoryForm,
-      opts.followSymlinkedDirectories,
-      opts.caseInsensitive,
-    ]
-      .map(on => (on ? '1' : '0'))
-      .join('') + globPath
-  const walked = opts.walks?.get(walkKey)
-  if (walked !== undefined) return walked
-
   const walk: GlobWalk = {
     baseLocation: '',
     matches: [],
@@ -1804,6 +1785,5 @@ export function* walkGlobPatternSteps(
     }
   }
 
-  opts.walks?.set(walkKey, walk)
   return walk
 }