Correct file hashing logic in the SDK (#916)

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
Co-authored-by: Jakub Dobry <jakub.dobry8@gmail.com>

Author: 3 people

.changeset/metal-penguins-begin.md

@@ -0,0 +1,6 @@
+---
+'@e2b/python-sdk': patch
+'e2b': patch
+---
+
+fix template build files copy cache invalidation

packages/js-sdk/src/template/buildApi.ts

@@ -1,8 +1,8 @@
-import { ApiClient, paths, handleApiError } from '../api'
+import { ApiClient, handleApiError, paths } from '../api'
+import { stripAnsi } from '../utils'
 import { BuildError, FileUploadError } from './errors'
 import { LogEntry } from './types'
 import { getBuildStepIndex, tarFileStreamUpload } from './utils'
-import { stripAnsi } from '../utils'
 
 type RequestBuildInput = {
   alias: string
@@ -91,14 +91,16 @@ export async function uploadFile(
     fileName: string
     fileContextPath: string
     url: string
+    resolveSymlinks: boolean
   },
-  stackTrace?: string
+  stackTrace: string | undefined
 ) {
-  const { fileName, url, fileContextPath } = options
+  const { fileName, url, fileContextPath, resolveSymlinks } = options
   try {
     const { contentLength, uploadStream } = await tarFileStreamUpload(
       fileName,
-      fileContextPath
+      fileContextPath,
+      resolveSymlinks
     )
 
     // The compiler assumes this is Web fetch API, but it's actually Node.js fetch API

packages/js-sdk/src/template/consts.ts

@@ -8,3 +8,5 @@ export const BASE_STEP_NAME = 'base'
  * 3. Caller method (eg. copy(), fromImage(), etc.)
  */
 export const STACK_TRACE_DEPTH = 3
+
+export const RESOLVE_SYMLINKS = false

packages/js-sdk/src/template/index.ts

@@ -9,7 +9,7 @@ import {
   uploadFile,
   waitForBuildFinish,
 } from './buildApi'
-import { STACK_TRACE_DEPTH } from './consts'
+import { RESOLVE_SYMLINKS, STACK_TRACE_DEPTH } from './consts'
 import { parseDockerfile } from './dockerfileParser'
 import { ReadyCmd } from './readycmd'
 import {
@@ -228,18 +228,30 @@ export class TemplateBase
   copy(
     src: string,
     dest: string,
-    options?: { forceUpload?: true; user?: string; mode?: number }
-  ): TemplateBuilder
-  copy(
-    items: CopyItem[],
-    options?: { forceUpload?: true; user?: string; mode?: number }
+    options?: {
+      forceUpload?: true
+      user?: string
+      mode?: number
+      resolveSymlinks?: boolean
+    }
   ): TemplateBuilder
+  copy(items: CopyItem[]): TemplateBuilder
   copy(
     srcOrItems: string | CopyItem[],
     destOrOptions?:
       | string
-      | { forceUpload?: true; user?: string; mode?: number },
-    options?: { forceUpload?: true; user?: string; mode?: number }
+      | {
+          forceUpload?: true
+          user?: string
+          mode?: number
+          resolveSymlinks?: boolean
+        },
+    options?: {
+      forceUpload?: true
+      user?: string
+      mode?: number
+      resolveSymlinks?: boolean
+    }
   ): TemplateBuilder {
     if (runtime === 'browser') {
       throw new Error('Browser runtime is not supported for copy')
@@ -254,6 +266,7 @@ export class TemplateBase
             mode: options?.mode,
             user: options?.user,
             forceUpload: options?.forceUpload,
+            resolveSymlinks: options?.resolveSymlinks,
           },
         ]
     for (const item of items) {
@@ -269,6 +282,7 @@ export class TemplateBase
         args,
         force: item.forceUpload ?? this.forceNextLayer,
         forceUpload: item.forceUpload,
+        resolveSymlinks: item.resolveSymlinks,
       })
     }
 
@@ -610,6 +624,7 @@ export class TemplateBase
               fileName: src,
               fileContextPath: this.fileContextPath,
               url,
+              resolveSymlinks: instruction.resolveSymlinks ?? RESOLVE_SYMLINKS,
             },
             stackTrace
           )
@@ -689,6 +704,7 @@ export class TemplateBase
                 ? []
                 : readDockerignore(this.fileContextPath)),
             ],
+            instruction.resolveSymlinks ?? RESOLVE_SYMLINKS,
             stackTrace
           ),
         }

packages/js-sdk/src/template/types.ts

@@ -15,6 +15,7 @@ export type Instruction = {
   force: boolean
   forceUpload?: boolean
   filesHash?: string
+  resolveSymlinks?: boolean
 }
 
 export type CopyItem = {
@@ -23,6 +24,7 @@ export type CopyItem = {
   forceUpload?: boolean
   user?: string
   mode?: number
+  resolveSymlinks?: boolean
 }
 
 // Interface for the initial state
@@ -75,13 +77,15 @@ export interface TemplateBuilder {
   copy(
     src: string,
     dest: string,
-    options?: { forceUpload?: true; user?: string; mode?: number }
+    options?: {
+      forceUpload?: true
+      user?: string
+      mode?: number
+      resolveSymlinks?: boolean
+    }
   ): TemplateBuilder
 
-  copy(
-    items: CopyItem[],
-    options?: { forceUpload?: true; user?: string; mode?: number }
-  ): TemplateBuilder
+  copy(items: CopyItem[]): TemplateBuilder
 
   remove(
     path: string,

packages/js-sdk/src/template/utils.ts

@@ -21,8 +21,9 @@ export async function calculateFilesHash(
   src: string,
   dest: string,
   contextPath: string,
-  ignorePatterns?: string[],
-  stackTrace?: string
+  ignorePatterns: string[],
+  resolveSymlinks: boolean,
+  stackTrace: string | undefined
 ): Promise<string> {
   const { glob } = await dynamicGlob()
   const srcPath = path.join(contextPath, src)
@@ -44,13 +45,47 @@ export async function calculateFilesHash(
     throw error
   }
 
+  // Hash stats
+  const hashStats = (stats: fs.Stats) => {
+    hash.update(stats.mode.toString())
+    hash.update(stats.uid.toString())
+    hash.update(stats.gid.toString())
+    hash.update(stats.size.toString())
+    hash.update(stats.mtimeMs.toString())
+  }
+
   for (const file of files) {
-    if (!file.isFile()) {
-      continue
+    // Add a relative path to hash calculation
+    const relativePath = path.relative(contextPath, file.fullpath())
+    hash.update(relativePath)
+
+    // Add stat information to hash calculation
+    if (file.isSymbolicLink()) {
+      // If the symlink is broken, it will return undefined, otherwise it will return a stats object of the target
+      const stats = fs.statSync(file.fullpath(), { throwIfNoEntry: false })
+      const shouldFollow =
+        resolveSymlinks && (stats?.isFile() || stats?.isDirectory())
+
+      if (!shouldFollow) {
+        const stats = fs.lstatSync(file.fullpath())
+
+        hashStats(stats)
+
+        const content = fs.readlinkSync(file.fullpath())
+        hash.update(content)
+
+        continue
+      }
     }
 
-    const content = fs.readFileSync(file.fullpath())
-    hash.update(new Uint8Array(content))
+    const stats = fs.statSync(file.fullpath())
+
+    hashStats(stats)
+
+    if (stats.isFile()) {
+      const content = fs.readFileSync(file.fullpath())
+      hash.update(new Uint8Array(content))
+    }
   }
 
   return hash.digest('hex')
@@ -105,34 +140,48 @@ export function padOctal(mode: number): string {
   return mode.toString(8).padStart(4, '0')
 }
 
-export async function tarFileStream(fileName: string, fileContextPath: string) {
+export async function tarFileStream(
+  fileName: string,
+  fileContextPath: string,
+  resolveSymlinks: boolean
+) {
   const { globSync } = await dynamicGlob()
   const { create } = await dynamicTar()
-  const files = globSync(fileName, { cwd: fileContextPath, nodir: false })
+  const files = globSync(fileName, { cwd: fileContextPath })
 
   return create(
     {
       gzip: true,
       cwd: fileContextPath,
+      follow: resolveSymlinks,
     },
     files
   )
 }
 
 export async function tarFileStreamUpload(
   fileName: string,
-  fileContextPath: string
+  fileContextPath: string,
+  resolveSymlinks: boolean
 ) {
   // First pass: calculate the compressed size without buffering
-  const sizeCalculationStream = await tarFileStream(fileName, fileContextPath)
+  const sizeCalculationStream = await tarFileStream(
+    fileName,
+    fileContextPath,
+    resolveSymlinks
+  )
   let contentLength = 0
   for await (const chunk of sizeCalculationStream as unknown as AsyncIterable<Buffer>) {
     contentLength += chunk.length
   }
 
   return {
     contentLength,
-    uploadStream: await tarFileStream(fileName, fileContextPath),
+    uploadStream: await tarFileStream(
+      fileName,
+      fileContextPath,
+      resolveSymlinks
+    ),
   }
 }
 

packages/js-sdk/tests/setup.ts

@@ -1,13 +1,27 @@
-import { Sandbox } from '../src'
+import { Sandbox, Template, TemplateClass } from '../src'
 import { test as base } from 'vitest'
 import { template } from './template'
+import { randomUUID } from 'node:crypto'
 
 interface SandboxFixture {
   sandbox: Sandbox
   template: string
   sandboxTestId: string
 }
 
+interface BuildTemplateFixture {
+  buildTemplate: (template: TemplateClass) => Promise<void>
+}
+
+function buildTemplate(template: TemplateClass, skipCache?: boolean) {
+  return Template.build(template, {
+    alias: randomUUID(),
+    cpuCount: 1,
+    memoryMB: 1024,
+    skipCache: skipCache,
+  })
+}
+
 export const sandboxTest = base.extend<SandboxFixture>({
   template,
   sandboxTestId: [
@@ -41,6 +55,16 @@ export const sandboxTest = base.extend<SandboxFixture>({
   ],
 })
 
+export const buildTemplateTest = base.extend<BuildTemplateFixture>({
+  buildTemplate: [
+    // eslint-disable-next-line no-empty-pattern
+    async ({}, use) => {
+      await use(buildTemplate)
+    },
+    { auto: true },
+  ],
+})
+
 export const isDebug = process.env.E2B_DEBUG !== undefined
 export const isIntegrationTest = process.env.E2B_INTEGRATION_TEST !== undefined