fix(cm-client): preserve broken symlinks when zipping repositories

Replace zip-lib archiveFolder with direct yazl usage in zipRepository
to handle broken (dangling) symlinks. zip-lib calls fs.stat() on symlink
targets which throws ENOENT for broken symlinks, failing the entire code
import. The new approach uses lstat to preserve symlinks as-is in the zip.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: rpapani

packages/spacecat-shared-cloud-manager-client/package.json

@@ -38,6 +38,7 @@
     "@adobe/spacecat-shared-ims-client": "1.11.6",
     "@adobe/spacecat-shared-utils": "1.81.1",
     "@aws-sdk/client-s3": "3.1014.0",
+    "yazl": "3.3.1",
     "zip-lib": "1.2.3"
   },
   "devDependencies": {

packages/spacecat-shared-cloud-manager-client/src/index.js

@@ -12,15 +12,17 @@
 
 import { execFileSync } from 'child_process';
 import {
-  existsSync, mkdtempSync, readdirSync, readFileSync,
+  createWriteStream,
+  existsSync, lstatSync, mkdtempSync, readdirSync, readFileSync,
   readlinkSync, rmSync, statfsSync, writeFileSync,
 } from 'fs';
 import os from 'os';
 import path from 'path';
 import { hasText, tracingFetch as fetch } from '@adobe/spacecat-shared-utils';
 import { ImsClient } from '@adobe/spacecat-shared-ims-client';
 import { GetObjectCommand } from '@aws-sdk/client-s3';
-import { archiveFolder, extract } from 'zip-lib';
+import { extract } from 'zip-lib';
+import yazl from 'yazl';
 
 const GIT_BIN = process.env.GIT_BIN_PATH || '/opt/bin/git';
 const CLONE_DIR_PREFIX = 'cm-repo-';
@@ -314,6 +316,7 @@ export default class CloudManagerClient {
   /**
    * Recursively validates that all symlinks under rootDir point to targets
    * within rootDir. Throws if any symlink escapes the root boundary.
+   * Logs a warning for broken symlinks (target does not exist).
    * @param {string} dir - Directory to scan
    * @param {string} rootDir - The root boundary all symlink targets must stay within
    */
@@ -329,12 +332,43 @@ export default class CloudManagerClient {
             `Symlink escapes repository root: ${path.relative(rootDir, fullPath)} -> ${target}`,
           );
         }
+        if (!existsSync(resolved)) {
+          this.log.warn(`Broken symlink: ${path.relative(rootDir, fullPath)} -> ${target} (target does not exist)`);
+        }
       } else if (entry.isDirectory()) {
         this.#validateSymlinks(fullPath, rootDir);
       }
     }
   }
 
+  /**
+   * Recursively walks a directory and adds all entries to a yazl ZipFile.
+   * Uses lstat (not stat) so broken symlinks are preserved as-is without
+   * following the link target.
+   * @param {import('yazl').ZipFile} zip - yazl ZipFile instance
+   * @param {string} dir - Current directory being walked
+   * @param {string} rootDir - Repository root (for computing relative paths)
+   */
+  #addDirToZip(zip, dir, rootDir) {
+    for (const entry of readdirSync(dir, { withFileTypes: true })) {
+      const fullPath = path.join(dir, entry.name);
+      const metadataPath = path.relative(rootDir, fullPath);
+
+      if (entry.isSymbolicLink()) {
+        const linkTarget = readlinkSync(fullPath);
+        const stat = lstatSync(fullPath);
+        zip.addBuffer(Buffer.from(linkTarget), metadataPath, {
+          mtime: stat.mtime,
+          mode: stat.mode,
+        });
+      } else if (entry.isDirectory()) {
+        this.#addDirToZip(zip, fullPath, rootDir);
+      } else {
+        zip.addFile(fullPath, metadataPath);
+      }
+    }
+  }
+
   /**
    * Zips the entire cloned repository including .git history.
    * Downstream services that read the ZIP from S3 need the full git history.
@@ -346,15 +380,30 @@ export default class CloudManagerClient {
       throw new Error(`Clone path does not exist: ${clonePath}`);
     }
 
-    // zip-lib is path-based (not buffer-based like adm-zip), so we write to
-    // a temp file and read the result back into a Buffer for the caller.
+    // yazl (and zip-lib) are path-based (not buffer-based like adm-zip), so we
+    // write to a temp file and read the result back into a Buffer for the caller.
     const zipDir = mkdtempSync(path.join(os.tmpdir(), ZIP_DIR_PREFIX));
     const zipFile = path.join(zipDir, 'repo.zip');
 
     try {
       this.log.info(`Zipping repository at ${clonePath}`);
       this.#validateSymlinks(clonePath, clonePath);
-      await archiveFolder(clonePath, zipFile, { followSymlinks: false });
+
+      // Use yazl directly instead of zip-lib's archiveFolder because zip-lib
+      // calls fs.stat() on symlink targets to determine file type, which fails
+      // for broken symlinks (ENOENT). yazl with lstat preserves symlinks as-is.
+      const zip = new yazl.ZipFile();
+      this.#addDirToZip(zip, clonePath, clonePath);
+      zip.end();
+
+      await new Promise((resolve, reject) => {
+        const output = createWriteStream(zipFile);
+        output.on('close', resolve);
+        output.on('error', reject);
+        zip.outputStream.on('error', reject);
+        zip.outputStream.pipe(output);
+      });
+
       this.#logTmpDiskUsage('zip');
       return readFileSync(zipFile);
     } catch (error) {

packages/spacecat-shared-cloud-manager-client/test/cloud-manager-client.test.js

@@ -12,6 +12,7 @@
 
 /* eslint-env mocha */
 
+import { EventEmitter } from 'events';
 import os from 'os';
 import path from 'path';
 import { expect, use } from 'chai';
@@ -97,25 +98,50 @@ function getGitArgsStr(call) {
 describe('CloudManagerClient', () => {
   let CloudManagerClient;
   const execFileSyncStub = sinon.stub();
+  const createWriteStreamStub = sinon.stub();
   const existsSyncStub = sinon.stub();
+  const lstatSyncStub = sinon.stub();
   const mkdtempSyncStub = sinon.stub();
   const readdirSyncStub = sinon.stub();
   const readFileSyncStub = sinon.stub().returns(Buffer.from('zip-content'));
   const readlinkSyncStub = sinon.stub();
   const rmSyncStub = sinon.stub();
   const statfsSyncStub = sinon.stub();
   const writeSyncStub = sinon.stub();
-  const archiveFolderStub = sinon.stub().resolves();
   const extractStub = sinon.stub().resolves();
 
+  // Mock yazl ZipFile — records all entries and simulates piping to a write stream
+  const mockZipEntries = [];
+  const mockYazlZipFile = {
+    addFile: sinon.stub().callsFake((filePath, metadataPath) => {
+      mockZipEntries.push({ type: 'file', filePath, metadataPath });
+    }),
+    addBuffer: sinon.stub().callsFake((buffer, metadataPath, opts) => {
+      mockZipEntries.push({
+        type: 'buffer', buffer, metadataPath, opts,
+      });
+    }),
+    end: sinon.stub(),
+    outputStream: {
+      on: sinon.stub().returnsThis(),
+      pipe: sinon.stub().callsFake((writable) => {
+        // Simulate async close after pipe
+        process.nextTick(() => writable.emit('close'));
+      }),
+    },
+  };
+  const YazlMock = { ZipFile: sinon.stub().returns(mockYazlZipFile) };
+
   // esmock's initial module resolution can exceed mocha's default 2s timeout
   // eslint-disable-next-line prefer-arrow-callback
   before(async function () {
     this.timeout(5000);
     const mod = await esmock('../src/index.js', {
       child_process: { execFileSync: execFileSyncStub },
       fs: {
+        createWriteStream: createWriteStreamStub,
         existsSync: existsSyncStub,
+        lstatSync: lstatSyncStub,
         mkdtempSync: mkdtempSyncStub,
         readdirSync: readdirSyncStub,
         readFileSync: readFileSyncStub,
@@ -124,7 +150,8 @@ describe('CloudManagerClient', () => {
         statfsSync: statfsSyncStub,
         writeFileSync: writeSyncStub,
       },
-      'zip-lib': { archiveFolder: archiveFolderStub, extract: extractStub },
+      'zip-lib': { extract: extractStub },
+      yazl: YazlMock,
     }, {
       '@adobe/spacecat-shared-ims-client': {
         ImsClient: { createFrom: createFromStub },
@@ -136,8 +163,16 @@ describe('CloudManagerClient', () => {
   beforeEach(() => {
     execFileSyncStub.reset();
     execFileSyncStub.returns('');
+    createWriteStreamStub.reset();
+    createWriteStreamStub.callsFake(() => {
+      const emitter = new EventEmitter();
+      emitter.write = sinon.stub();
+      emitter.end = sinon.stub();
+      return emitter;
+    });
     existsSyncStub.reset();
     existsSyncStub.returns(false);
+    lstatSyncStub.reset();
     mkdtempSyncStub.reset();
     mkdtempSyncStub.callsFake((prefix) => `${prefix}XXXXXX`);
     readdirSyncStub.reset();
@@ -149,10 +184,21 @@ describe('CloudManagerClient', () => {
     statfsSyncStub.reset();
     statfsSyncStub.returns({ bsize: 4096, blocks: 131072, bfree: 65536 });
     writeSyncStub.reset();
-    archiveFolderStub.reset();
-    archiveFolderStub.resolves();
     extractStub.reset();
     extractStub.resolves();
+    // Reset yazl mocks
+    mockZipEntries.length = 0;
+    YazlMock.ZipFile.reset();
+    YazlMock.ZipFile.returns(mockYazlZipFile);
+    mockYazlZipFile.addFile.reset();
+    mockYazlZipFile.addBuffer.reset();
+    mockYazlZipFile.end.reset();
+    mockYazlZipFile.outputStream.on.reset();
+    mockYazlZipFile.outputStream.on.returnsThis();
+    mockYazlZipFile.outputStream.pipe.reset();
+    mockYazlZipFile.outputStream.pipe.callsFake((writable) => {
+      process.nextTick(() => writable.emit('close'));
+    });
     createFromStub.reset();
     createFromStub.returns(mockImsClient);
     mockImsClient.getServiceAccessToken.reset();
@@ -878,10 +924,9 @@ describe('CloudManagerClient', () => {
       expect(mkdtempSyncStub).to.have.been.calledOnce;
       expect(mkdtempSyncStub.firstCall.args[0]).to.match(/cm-zip-$/);
 
-      // Should archive the folder with followSymlinks: false
-      expect(archiveFolderStub).to.have.been.calledOnce;
-      expect(archiveFolderStub.firstCall.args[0]).to.equal(clonePath);
-      expect(archiveFolderStub.firstCall.args[2]).to.deep.equal({ followSymlinks: false });
+      // Should use yazl to create the zip
+      expect(YazlMock.ZipFile).to.have.been.calledOnce;
+      expect(mockYazlZipFile.end).to.have.been.calledOnce;
 
       // Should read the zip file into a buffer
       expect(readFileSyncStub).to.have.been.calledOnce;
@@ -907,23 +952,133 @@ describe('CloudManagerClient', () => {
       await expect(client.zipRepository(clonePath))
         .to.be.rejectedWith('Symlink escapes repository root: evil-link -> /etc/passwd');
 
-      // archiveFolder should never be called
-      expect(archiveFolderStub).to.not.have.been.called;
+      // yazl should never be used
+      expect(YazlMock.ZipFile).to.not.have.been.called;
 
       // Should still clean up the temp zip directory
       expect(rmSyncStub).to.have.been.calledOnce;
       expect(rmSyncStub.firstCall.args[0]).to.match(/cm-zip-/);
     });
 
-    it('throws when archiveFolder fails and cleans up temp dir', async () => {
+    it('preserves broken symlinks as-is in the zip', async () => {
+      const clonePath = '/tmp/cm-repo-zip-test';
+      existsSyncStub.withArgs(clonePath).returns(true);
+
+      const enabledFarmsPath = path.join(clonePath, 'dispatcher', 'enabled_farms');
+      const brokenLinkPath = path.join(enabledFarmsPath, 'broken.farm');
+      const brokenTarget = '../available_farms/missing.farm';
+      const resolvedTarget = path.resolve(enabledFarmsPath, brokenTarget);
+      const symlinkMtime = new Date('2025-01-01');
+      const symlinkMode = 0o120777;
+
+      // Root dir has a 'dispatcher' directory
+      readdirSyncStub.withArgs(clonePath, { withFileTypes: true }).returns([{
+        name: 'dispatcher',
+        isSymbolicLink: () => false,
+        isDirectory: () => true,
+      }]);
+      // dispatcher has 'enabled_farms' directory
+      readdirSyncStub.withArgs(path.join(clonePath, 'dispatcher'), { withFileTypes: true }).returns([{
+        name: 'enabled_farms',
+        isSymbolicLink: () => false,
+        isDirectory: () => true,
+      }]);
+      // enabled_farms has a broken symlink
+      readdirSyncStub.withArgs(enabledFarmsPath, { withFileTypes: true }).returns([{
+        name: 'broken.farm',
+        isSymbolicLink: () => true,
+        isDirectory: () => false,
+      }]);
+      readlinkSyncStub.withArgs(brokenLinkPath).returns(brokenTarget);
+      lstatSyncStub.withArgs(brokenLinkPath).returns({ mtime: symlinkMtime, mode: symlinkMode });
+      // Target does not exist — broken symlink
+      existsSyncStub.withArgs(resolvedTarget).returns(false);
+
+      const ctx = createContext();
+      const client = CloudManagerClient.createFrom(ctx);
+      const result = await client.zipRepository(clonePath);
+
+      expect(Buffer.isBuffer(result)).to.be.true;
+
+      // Should log a warning about the broken symlink
+      expect(ctx.log.warn).to.have.been.calledWithMatch(/Broken symlink.*broken\.farm.*missing\.farm/);
+
+      // Should add the symlink via addBuffer with symlink mode bits preserved
+      expect(mockYazlZipFile.addBuffer).to.have.been.calledOnce;
+      const [buf, metadataPath, opts] = mockYazlZipFile.addBuffer.firstCall.args;
+      expect(buf.toString()).to.equal(brokenTarget);
+      expect(metadataPath).to.equal(path.relative(clonePath, brokenLinkPath));
+      expect(opts.mode).to.equal(symlinkMode);
+      expect(opts.mtime).to.equal(symlinkMtime);
+    });
+
+    it('adds regular files via yazl addFile', async () => {
       const clonePath = '/tmp/cm-repo-zip-test';
       existsSyncStub.withArgs(clonePath).returns(true);
-      archiveFolderStub.rejects(new Error('failed to read directory'));
+
+      readdirSyncStub.withArgs(clonePath, { withFileTypes: true }).returns([{
+        name: 'index.html',
+        isSymbolicLink: () => false,
+        isDirectory: () => false,
+      }]);
+
+      const client = CloudManagerClient.createFrom(createContext());
+      await client.zipRepository(clonePath);
+
+      expect(mockYazlZipFile.addFile).to.have.been.calledOnce;
+      expect(mockYazlZipFile.addFile.firstCall.args[0]).to.equal(path.join(clonePath, 'index.html'));
+      expect(mockYazlZipFile.addFile.firstCall.args[1]).to.equal('index.html');
+    });
+
+    it('adds valid symlinks via addBuffer with lstat mode', async () => {
+      const clonePath = '/tmp/cm-repo-zip-test';
+      existsSyncStub.withArgs(clonePath).returns(true);
+
+      const subDir = path.join(clonePath, 'enabled');
+      const linkPath = path.join(subDir, 'link.farm');
+      const linkTarget = '../available/target.farm';
+      const resolvedTarget = path.resolve(subDir, linkTarget);
+
+      readdirSyncStub.withArgs(clonePath, { withFileTypes: true }).returns([{
+        name: 'enabled',
+        isSymbolicLink: () => false,
+        isDirectory: () => true,
+      }]);
+      readdirSyncStub.withArgs(subDir, { withFileTypes: true }).returns([{
+        name: 'link.farm',
+        isSymbolicLink: () => true,
+        isDirectory: () => false,
+      }]);
+      readlinkSyncStub.withArgs(linkPath).returns(linkTarget);
+      lstatSyncStub.withArgs(linkPath).returns({ mtime: new Date(), mode: 0o120777 });
+      existsSyncStub.withArgs(resolvedTarget).returns(true);
+
+      const client = CloudManagerClient.createFrom(createContext());
+      await client.zipRepository(clonePath);
+
+      // Symlink stored as buffer with link target content
+      expect(mockYazlZipFile.addBuffer).to.have.been.calledOnce;
+      expect(mockYazlZipFile.addBuffer.firstCall.args[0].toString()).to.equal(linkTarget);
+    });
+
+    it('throws when yazl zip fails and cleans up temp dir', async () => {
+      const clonePath = '/tmp/cm-repo-zip-test';
+      existsSyncStub.withArgs(clonePath).returns(true);
+
+      // Make the output stream emit an error
+      mockYazlZipFile.outputStream.pipe.callsFake((_) => {
+        process.nextTick(() => {
+          // Emit error on the outputStream error handler
+          const errorHandler = mockYazlZipFile.outputStream.on.getCalls()
+            .find((c) => c.args[0] === 'error');
+          if (errorHandler) errorHandler.args[1](new Error('write failed'));
+        });
+      });
 
       const client = CloudManagerClient.createFrom(createContext());
 
       await expect(client.zipRepository(clonePath))
-        .to.be.rejectedWith('Failed to zip repository: failed to read directory');
+        .to.be.rejectedWith('Failed to zip repository: write failed');
 
       // Should still clean up the temp zip directory
       expect(rmSyncStub).to.have.been.calledOnce;