From 5bc072a32f95b50075af8d535c209bcfbfcc8788 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Wed, 9 Sep 2026 11:23:59 -0600 Subject: [PATCH 1/8] fix: prevent symlink following during partial-delete in retrieve @W-24124120@ Use lstatSync instead of statSync to reject symlinked content directories from partial-delete processing. Filter out symlinks within content directories before deletion. Add defense-in-depth symlink check in deleteFilePath. --- src/client/retrieveExtract.ts | 17 ++++++++- test/client/retrieveExtract.test.ts | 59 +++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/client/retrieveExtract.ts b/src/client/retrieveExtract.ts index 57805f0471..1993134cff 100644 --- a/src/client/retrieveExtract.ts +++ b/src/client/retrieveExtract.ts @@ -179,6 +179,7 @@ const handlePartialDeleteMerges = ({ return matchingLocalComp.contentList .filter((fileName) => !remoteContentList.has(fileName)) .filter((fileName) => !pathOrSomeChildIsIgnored(logger)(comp)(matchingLocalComp)(fileName)) + .filter((fileName) => !isSymlinkSync(path.join(matchingLocalComp.contentPath, fileName))) .map( (fileName): FileResponseSuccess => ({ fullName: comp.fullName, @@ -192,7 +193,7 @@ const handlePartialDeleteMerges = ({ }; const supportsPartialDeleteAndHasContent = (comp: SourceComponent): comp is SourceComponentWithContent => - supportsPartialDelete(comp) && typeof comp.content === 'string' && fs.statSync(comp.content).isDirectory(); + supportsPartialDelete(comp) && typeof comp.content === 'string' && fs.lstatSync(comp.content).isDirectory(); const supportsPartialDeleteAndHasZipContent = (tree: ZipTreeContainer) => @@ -220,7 +221,7 @@ const pathOrSomeChildIsIgnored = (localComp: PartialDeleteComp) => (fileName: string): boolean => { const fileNameFullPath = path.join(localComp.contentPath, fileName); - return fs.statSync(fileNameFullPath).isDirectory() + return fs.lstatSync(fileNameFullPath).isDirectory() ? fs.readdirSync(fileNameFullPath).map(fnJoin(fileNameFullPath)).some(isForceIgnored(logger)(component)) : isForceIgnored(logger)(component)(fileNameFullPath); }; @@ -236,10 +237,22 @@ const isForceIgnored = return ignored; }; +const isSymlinkSync = (filePath: string): boolean => { + try { + return fs.lstatSync(filePath).isSymbolicLink(); + } catch { + return false; + } +}; + const deleteFilePath = (logger: Logger) => (fr: FileResponseSuccess): FileResponseSuccess => { if (fr.filePath) { + if (isSymlinkSync(fr.filePath)) { + logger.debug(`Skipping delete of symlink ${fr.filePath} to prevent modification of files outside the project.`); + return fr; + } logger.debug( `Local component (${fr.fullName}) contains ${fr.filePath} while remote component does not. This file is being removed.` ); diff --git a/test/client/retrieveExtract.test.ts b/test/client/retrieveExtract.test.ts index 59c69d1d20..8167bf7b5d 100644 --- a/test/client/retrieveExtract.test.ts +++ b/test/client/retrieveExtract.test.ts @@ -14,6 +14,8 @@ * limitations under the License. */ import { join } from 'node:path'; +import os from 'node:os'; +import fs from 'graceful-fs'; import { expect } from 'chai'; import { XMLParser } from 'fast-xml-parser'; import { registry, RegistryAccess, SourceComponent, VirtualTreeContainer } from '../../src'; @@ -874,3 +876,60 @@ describe('retrieveExtract - Version Filtering', () => { }); }); }); + +describe('partial-delete symlink protection', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(join(os.tmpdir(), 'sdr-symlink-test-')); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('lstatSync rejects symlinked directories (used by supportsPartialDeleteAndHasContent)', () => { + const realDir = join(tmpDir, 'real'); + fs.mkdirSync(realDir); + const symlinkDir = join(tmpDir, 'link'); + fs.symlinkSync(realDir, symlinkDir); + + // statSync follows symlinks — would treat a symlink-to-directory as a directory (the old vulnerable behavior) + expect(fs.statSync(symlinkDir).isDirectory()).to.be.true; + // lstatSync does NOT follow — sees the symlink itself, not a directory (the fix) + expect(fs.lstatSync(symlinkDir).isDirectory()).to.be.false; + expect(fs.lstatSync(symlinkDir).isSymbolicLink()).to.be.true; + }); + + it('lstatSync still recognizes regular directories (no false positives)', () => { + const realDir = join(tmpDir, 'real'); + fs.mkdirSync(realDir); + + expect(fs.lstatSync(realDir).isDirectory()).to.be.true; + expect(fs.lstatSync(realDir).isSymbolicLink()).to.be.false; + }); + + it('lstatSync detects symlinked files within a content directory', () => { + const contentDir = join(tmpDir, 'content'); + fs.mkdirSync(contentDir); + const externalFile = join(tmpDir, 'external.txt'); + fs.writeFileSync(externalFile, 'external data'); + const symlinkFile = join(contentDir, 'linked.txt'); + fs.symlinkSync(externalFile, symlinkFile); + + expect(fs.lstatSync(symlinkFile).isSymbolicLink()).to.be.true; + }); + + it('rmSync on a symlink removes the link, not the target', () => { + const externalDir = join(tmpDir, 'external'); + fs.mkdirSync(externalDir); + fs.writeFileSync(join(externalDir, 'important.txt'), 'do not delete'); + const symlinkPath = join(tmpDir, 'link'); + fs.symlinkSync(externalDir, symlinkPath); + + fs.rmSync(symlinkPath, { force: true }); + + expect(fs.existsSync(symlinkPath)).to.be.false; + expect(fs.existsSync(join(externalDir, 'important.txt'))).to.be.true; + }); +}); From 9f48787e84480041eccd914276a3857510850945 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Wed, 9 Sep 2026 11:30:40 -0600 Subject: [PATCH 2/8] build: bump @salesforce/core to ^9.1.11 --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 8f31fff20f..bb3b146e41 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "node": ">=22.0.0" }, "dependencies": { - "@salesforce/core": "^9.1.10", + "@salesforce/core": "^9.1.11", "@salesforce/kit": "^4.0.0", "@salesforce/ts-types": "^3.2.0", "@salesforce/types": "^1.6.0", diff --git a/yarn.lock b/yarn.lock index 05e57ceb85..6cfa31fdf8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -845,10 +845,10 @@ ts-retry-promise "^0.8.1" zod "^4.1.12" -"@salesforce/core@^9.1.10": - version "9.1.10" - resolved "https://registry.yarnpkg.com/@salesforce/core/-/core-9.1.10.tgz#a3a01455c33821f5f4af4f85ba09e4b18f55d194" - integrity sha512-0BRMIUHU21jOhK93tNEt9jtXaFnhwoNuRZynnVuuaNIogF5b2X8L6GYRXTqBHYPtX/MHqpKvAVMs6oMj3LcIpw== +"@salesforce/core@^9.1.11": + version "9.1.11" + resolved "https://registry.yarnpkg.com/@salesforce/core/-/core-9.1.11.tgz#dd2226ac1d67434575aac92eaa3ad9c1a12dcc2e" + integrity sha512-EE4oGYsoKThank4dQ1mVIzJjS9yP6C6s7JzKDx+ki8AFAxgpposzLc6GYwZZq1tS0XaOfBL2x+8RrgEgZvXwnw== dependencies: "@jsforce/jsforce-node" "^3.10.24" "@salesforce/kit" "^4.0.0" From 69e18c475cb55d63ead1b397fbdafcd37e20a4b2 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Fri, 11 Sep 2026 13:05:45 -0600 Subject: [PATCH 3/8] fix: prevent symlink following during retrieve writes @W-24138711@ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract findSymlinkOnPath to shared fileSystemHandler utility and add symlink traversal protection to StandardWriter._write() — covers both the write and delete paths for all metadata types during retrieve. --- eslint-suppressions.json | 4 +- messages/sdr.md | 4 ++ src/convert/streams.ts | 19 ++++++- .../staticResourceMetadataTransformer.ts | 32 ++---------- src/utils/fileSystemHandler.ts | 24 +++++++++ test/convert/streams.test.ts | 46 ++++++++++++++++ test/utils/fileSystemHandler.test.ts | 52 ++++++++++++++++++- 7 files changed, 147 insertions(+), 34 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index e9da1915a1..7f15a817fe 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -188,7 +188,7 @@ "count": 5 }, "no-underscore-dangle": { - "count": 25 + "count": 28 } }, "test/convert/transformers/decomposedLabelsTransformer.test.ts": { @@ -360,4 +360,4 @@ "count": 1 } } -} \ No newline at end of file +} diff --git a/messages/sdr.md b/messages/sdr.md index 8b7b270d4d..8b6380096b 100644 --- a/messages/sdr.md +++ b/messages/sdr.md @@ -117,6 +117,10 @@ Entry '%s' in static resource '%s' resolves to a location outside the extraction Entry '%s' in static resource '%s' would be written through a symbolic link ('%s'). Writing through symbolic links is not allowed because it can place files outside the extraction directory ('%s'). +# error_retrieve_symlink + +File '%s' would be written through a symbolic link ('%s'). Writing through symbolic links is not allowed during retrieve because it can place files outside the project directory ('%s'). + # error_static_resource_expected_archive_type A StaticResource directory must have a content type of application/zip or application/jar - found %s for %s. diff --git a/src/convert/streams.ts b/src/convert/streams.ts index 54cf9293c1..91c7600bba 100644 --- a/src/convert/streams.ts +++ b/src/convert/streams.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { isAbsolute, join } from 'node:path'; +import { isAbsolute, join, relative } from 'node:path'; import { pipeline as cbPipeline, Readable, Transform, Writable, Stream } from 'node:stream'; import { promisify } from 'node:util'; import JSZip from 'jszip'; @@ -21,18 +21,22 @@ import { createWriteStream, existsSync, promises as fsPromises } from 'graceful- import { JsonMap } from '@salesforce/ts-types'; import { XMLBuilder } from 'fast-xml-parser'; import { Logger } from '@salesforce/core/logger'; +import { Messages } from '@salesforce/core/messages'; import { SourceComponent } from '../resolve/sourceComponent'; import { SourcePath } from '../common/types'; import { XML_COMMENT_PROP_NAME, XML_DECL } from '../common/constants'; import { ComponentSet } from '../collections/componentSet'; import { RegistryAccess } from '../registry/registryAccess'; -import { ensureFileExists } from '../utils/fileSystemHandler'; +import { ensureFileExists, findSymlinkOnPath } from '../utils/fileSystemHandler'; import { ComponentStatus, FileResponseSuccess } from '../client/types'; import { ForceIgnore } from '../resolve'; import { MetadataTransformerFactory } from './transformers/metadataTransformerFactory'; import { ConvertContext } from './convertContext/convertContext'; import { SfdxFileFormat, WriteInfo, WriterFormat } from './types'; +Messages.importMessagesDirectory(__dirname); +const messages = Messages.loadMessages('@salesforce/source-deploy-retrieve', 'sdr'); + export type PromisifiedPipeline = ( source: T, ...destinations: NodeJS.WritableStream[] @@ -163,6 +167,17 @@ export class StandardWriter extends ComponentWriter { .map(makeWriteInfoAbsolute(this.rootDestination)) .filter(existsOrDoesntMatchIgnored(this.forceignore, this.logger)) // Skip files matched by default ignore .map(async (info) => { + if (this.rootDestination) { + const symlink = await findSymlinkOnPath(this.rootDestination, info.output); + if (symlink) { + throw messages.createError('error_retrieve_symlink', [ + info.output, + relative(this.rootDestination, symlink), + this.rootDestination, + ]); + } + } + if (info.shouldDelete) { this.deleted.push({ filePath: info.output, diff --git a/src/convert/transformers/staticResourceMetadataTransformer.ts b/src/convert/transformers/staticResourceMetadataTransformer.ts index 4fd3b8add8..6600bb2ff3 100644 --- a/src/convert/transformers/staticResourceMetadataTransformer.ts +++ b/src/convert/transformers/staticResourceMetadataTransformer.ts @@ -13,12 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { basename, dirname, isAbsolute, join, relative, sep } from 'node:path'; +import { basename, dirname, isAbsolute, join, relative } from 'node:path'; import { Readable } from 'node:stream'; import JSZip from 'jszip'; import { getExtension } from 'mime'; import { JsonMap } from '@salesforce/ts-types'; -import { createWriteStream, promises as fs } from 'graceful-fs'; +import { createWriteStream } from 'graceful-fs'; import { Logger } from '@salesforce/core/logger'; import { Messages } from '@salesforce/core/messages'; import { SfError } from '@salesforce/core/sfError'; @@ -27,7 +27,7 @@ import { baseName } from '../../utils/path'; import { ToSourceFormatInput, WriteInfo } from '../types'; import { SourceComponent } from '../../resolve/sourceComponent'; import { SourcePath } from '../../common/types'; -import { ensureFileExists } from '../../utils/fileSystemHandler'; +import { ensureFileExists, findSymlinkOnPath } from '../../utils/fileSystemHandler'; import { getPipeline } from '../streams'; import { getReplacementStreamForReadable } from '../replacements'; import { BaseMetadataTransformer } from './baseMetadataTransformer'; @@ -271,32 +271,6 @@ const componentIsExpandedArchive = async (component: SourceComponent): Promise => { - const rel = relative(root, destination); - const segments = rel.split(sep).filter((s) => s.length > 0); - // Build the cumulative path for each segment below the root, then lstat them concurrently. - const paths = segments.map((_, i) => join(root, ...segments.slice(0, i + 1))); - const results = await Promise.all( - paths.map(async (p) => { - try { - return (await fs.lstat(p)).isSymbolicLink(); - } catch { - // Path segment does not exist yet; it will be created as a regular file/dir, so it is safe. - return false; - } - }) - ); - return paths.find((_, i) => results[i]); -}; - async function getStaticResourceZip(component: SourceComponent, content: string): Promise { try { const staticResourceZip = await component.tree.readFile(content); diff --git a/src/utils/fileSystemHandler.ts b/src/utils/fileSystemHandler.ts index 3fcca58566..973a1d39a4 100644 --- a/src/utils/fileSystemHandler.ts +++ b/src/utils/fileSystemHandler.ts @@ -45,3 +45,27 @@ export function searchUp(start: SourcePath, fileName: string): string | undefine return searchUp(parent, fileName); } + +/** + * Walk every path segment between the root (exclusive) and the destination + * (inclusive) and return the first one that is a symbolic link, or undefined if none is. + * + * Both `createWriteStream` and recursive `mkdir` follow symlinks, so a link planted anywhere + * along the destination path can redirect writes outside the root. The root is assumed trusted + * and is not checked. + */ +export const findSymlinkOnPath = async (root: string, destination: string): Promise => { + const rel = path.relative(root, destination); + const segments = rel.split(path.sep).filter((s) => s.length > 0); + const paths = segments.map((_, i) => path.join(root, ...segments.slice(0, i + 1))); + const results = await Promise.all( + paths.map(async (p) => { + try { + return (await fs.promises.lstat(p)).isSymbolicLink(); + } catch { + return false; + } + }) + ); + return paths.find((_, i) => results[i]); +}; diff --git a/test/convert/streams.test.ts b/test/convert/streams.test.ts index 1c0236e183..e97f6a2a54 100644 --- a/test/convert/streams.test.ts +++ b/test/convert/streams.test.ts @@ -278,10 +278,12 @@ describe('Streams', () => { let writer: streams.StandardWriter; let ensureFile: SinonStub; + let findSymlinkStub: SinonStub; beforeEach(() => { writer = new streams.StandardWriter(rootDestination); ensureFile = env.stub(fsUtil, 'ensureFileExists'); + findSymlinkStub = env.stub(fsUtil, 'findSymlinkOnPath').resolves(undefined); const mockPipeline = env.stub().resolves(); pipelineStub = env.stub(streams, 'getPipeline').returns(mockPipeline); env @@ -451,6 +453,50 @@ describe('Streams', () => { expect(loggerStub.firstCall.args[0]).to.equal(expectedLogMsg); }); }); + + it('should throw when a symlink is detected on the write path', async () => { + const symlinkPath = join(rootDestination, COMPONENT.type.directoryName); + findSymlinkStub.resolves(symlinkPath); + + await writer._write(chunk, '', (err: Error | undefined) => { + assert(err instanceof Error); + expect(err.message).to.include('symbolic link'); + }); + }); + + it('should throw when a symlink is detected on the delete path', async () => { + const deleteChunk: WriterFormat = { + component, + writeInfos: [ + { + output: component.getPackageRelativePath(component.xml!, 'metadata'), + shouldDelete: true, + type: component.type.name, + fullName: component.fullName, + }, + ], + }; + const symlinkPath = join(rootDestination, COMPONENT.type.directoryName); + findSymlinkStub.resolves(symlinkPath); + + await writer._write(deleteChunk, '', (err: Error | undefined) => { + assert(err instanceof Error); + expect(err.message).to.include('symbolic link'); + }); + }); + + it('should call findSymlinkOnPath for each writeInfo', async () => { + const mockPipeline = env.stub().resolves(); + pipelineStub.returns(mockPipeline); + + await writer._write(chunk, '', (err: Error | undefined) => { + expect(err).to.be.undefined; + expect(findSymlinkStub.callCount).to.equal(chunk.writeInfos.length); + for (const call of findSymlinkStub.getCalls()) { + expect(call.args[0]).to.equal(rootDestination); + } + }); + }); }); describe('ZipWriter', () => { diff --git a/test/utils/fileSystemHandler.test.ts b/test/utils/fileSystemHandler.test.ts index ae50da9249..b2aaf95488 100644 --- a/test/utils/fileSystemHandler.test.ts +++ b/test/utils/fileSystemHandler.test.ts @@ -14,10 +14,11 @@ * limitations under the License. */ import { join } from 'node:path'; +import os from 'node:os'; import { SinonStub, createSandbox } from 'sinon'; import { expect, config } from 'chai'; import fs from 'graceful-fs'; -import { searchUp } from '../../src/utils/fileSystemHandler'; +import { searchUp, findSymlinkOnPath } from '../../src/utils/fileSystemHandler'; const env = createSandbox(); config.truncateThreshold = 0; @@ -51,4 +52,53 @@ describe('File System Utils', () => { expect(searchUp(startPath, 'asdf')).to.be.undefined; }); }); + + describe('findSymlinkOnPath', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(join(os.tmpdir(), 'sdr-symlink-test-')); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('should return undefined when no symlinks exist', async () => { + const sub = join(tmpDir, 'a', 'b'); + fs.mkdirSync(sub, { recursive: true }); + const dest = join(sub, 'file.cls'); + fs.writeFileSync(dest, 'content'); + + expect(await findSymlinkOnPath(tmpDir, dest)).to.be.undefined; + }); + + it('should detect a symlinked file at the destination', async () => { + const external = join(tmpDir, 'external.txt'); + fs.writeFileSync(external, 'external'); + const sub = join(tmpDir, 'project'); + fs.mkdirSync(sub); + const link = join(sub, 'link.txt'); + fs.symlinkSync(external, link); + + expect(await findSymlinkOnPath(tmpDir, link)).to.equal(link); + }); + + it('should detect a symlinked directory in the path', async () => { + const externalDir = join(tmpDir, 'external'); + fs.mkdirSync(externalDir); + const project = join(tmpDir, 'project'); + fs.mkdirSync(project); + const linkedDir = join(project, 'classes'); + fs.symlinkSync(externalDir, linkedDir); + + const dest = join(linkedDir, 'MyClass.cls'); + expect(await findSymlinkOnPath(tmpDir, dest)).to.equal(linkedDir); + }); + + it('should return undefined when path segments do not exist yet', async () => { + const dest = join(tmpDir, 'nonexistent', 'deep', 'file.cls'); + expect(await findSymlinkOnPath(tmpDir, dest)).to.be.undefined; + }); + }); }); From 9f87e14d7d592242559aaed931a3ce4f475c425e Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Tue, 15 Sep 2026 08:17:07 -0600 Subject: [PATCH 4/8] fix: close ancestor-symlink gap in partial-delete path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The delete path only checked the leaf segment for symlinks, so a symlinked ancestor directory (e.g. digitalExperiences/ → external) could let rmSync delete files outside the project before the write-path guard fired. - Add findSymlinkOnPathSync to walk all segments from package root - Use it in the pre-filter (before FileResponse creation) and as defense-in-depth in deleteFilePath - Guard both findSymlinkOnPath variants against destinations that resolve outside the root (.. traversal) - Remove now-dead isSymlinkSync helper --- src/client/retrieveExtract.ts | 26 ++++++------ src/utils/fileSystemHandler.ts | 18 +++++++++ test/utils/fileSystemHandler.test.ts | 60 +++++++++++++++++++++++++++- 3 files changed, 89 insertions(+), 15 deletions(-) diff --git a/src/client/retrieveExtract.ts b/src/client/retrieveExtract.ts index 1993134cff..4667b73765 100644 --- a/src/client/retrieveExtract.ts +++ b/src/client/retrieveExtract.ts @@ -25,6 +25,7 @@ import { ComponentSet } from '../collections'; import { ZipTreeContainer } from '../resolve'; import { SourceComponent, SourceComponentWithContent } from '../resolve/sourceComponent'; import { fnJoin } from '../utils/path'; +import { findSymlinkOnPathSync } from '../utils/fileSystemHandler'; import { correctComments, handleSpecialEntities } from '../convert/streams'; import { BotVersionFilter, @@ -117,7 +118,7 @@ export const extract = async ({ if (merge) { partialDeleteFileResponses.push( - ...handlePartialDeleteMerges({ retrievedComponents, tree, mainComponents, logger }) + ...handlePartialDeleteMerges({ retrievedComponents, tree, mainComponents, logger, packageRoot: pkg.outputDir }) ); } @@ -148,11 +149,13 @@ const handlePartialDeleteMerges = ({ retrievedComponents, tree, logger, + packageRoot, }: { mainComponents?: ComponentSet; retrievedComponents: SourceComponent[]; tree: ZipTreeContainer; logger: Logger; + packageRoot: string; }): FileResponse[] => { // Find all merge (local) components that support partial delete. const partialDeleteComponents = new Map( @@ -179,7 +182,9 @@ const handlePartialDeleteMerges = ({ return matchingLocalComp.contentList .filter((fileName) => !remoteContentList.has(fileName)) .filter((fileName) => !pathOrSomeChildIsIgnored(logger)(comp)(matchingLocalComp)(fileName)) - .filter((fileName) => !isSymlinkSync(path.join(matchingLocalComp.contentPath, fileName))) + .filter( + (fileName) => !findSymlinkOnPathSync(packageRoot, path.join(matchingLocalComp.contentPath, fileName)) + ) .map( (fileName): FileResponseSuccess => ({ fullName: comp.fullName, @@ -188,7 +193,7 @@ const handlePartialDeleteMerges = ({ filePath: path.join(matchingLocalComp.contentPath, fileName), }) ) - .map(deleteFilePath(logger)); + .map(deleteFilePath(logger, packageRoot)); }); }; @@ -237,20 +242,13 @@ const isForceIgnored = return ignored; }; -const isSymlinkSync = (filePath: string): boolean => { - try { - return fs.lstatSync(filePath).isSymbolicLink(); - } catch { - return false; - } -}; - const deleteFilePath = - (logger: Logger) => + (logger: Logger, packageRoot: string) => (fr: FileResponseSuccess): FileResponseSuccess => { if (fr.filePath) { - if (isSymlinkSync(fr.filePath)) { - logger.debug(`Skipping delete of symlink ${fr.filePath} to prevent modification of files outside the project.`); + const symlink = findSymlinkOnPathSync(packageRoot, fr.filePath); + if (symlink) { + logger.debug(`Skipping delete of ${fr.filePath} — path segment ${symlink} is a symbolic link.`); return fr; } logger.debug( diff --git a/src/utils/fileSystemHandler.ts b/src/utils/fileSystemHandler.ts index 973a1d39a4..53a06a17ab 100644 --- a/src/utils/fileSystemHandler.ts +++ b/src/utils/fileSystemHandler.ts @@ -69,3 +69,21 @@ export const findSymlinkOnPath = async (root: string, destination: string): Prom ); return paths.find((_, i) => results[i]); }; + +/** Synchronous variant of {@link findSymlinkOnPath} for use in sync call-chains. */ +export const findSymlinkOnPathSync = (root: string, destination: string): string | undefined => { + const rel = path.relative(root, destination); + if (rel.startsWith('..')) return destination; + const segments = rel.split(path.sep).filter((s) => s.length > 0); + for (let i = 0; i < segments.length; i++) { + const p = path.join(root, ...segments.slice(0, i + 1)); + try { + if (fs.lstatSync(p).isSymbolicLink()) { + return p; + } + } catch { + // path segment doesn't exist, skip + } + } + return undefined; +}; diff --git a/test/utils/fileSystemHandler.test.ts b/test/utils/fileSystemHandler.test.ts index b2aaf95488..833a84ea87 100644 --- a/test/utils/fileSystemHandler.test.ts +++ b/test/utils/fileSystemHandler.test.ts @@ -18,7 +18,7 @@ import os from 'node:os'; import { SinonStub, createSandbox } from 'sinon'; import { expect, config } from 'chai'; import fs from 'graceful-fs'; -import { searchUp, findSymlinkOnPath } from '../../src/utils/fileSystemHandler'; +import { searchUp, findSymlinkOnPath, findSymlinkOnPathSync } from '../../src/utils/fileSystemHandler'; const env = createSandbox(); config.truncateThreshold = 0; @@ -101,4 +101,62 @@ describe('File System Utils', () => { expect(await findSymlinkOnPath(tmpDir, dest)).to.be.undefined; }); }); + + describe('findSymlinkOnPathSync', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(join(os.tmpdir(), 'sdr-symlink-sync-test-')); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('should return undefined when no symlinks exist', () => { + const sub = join(tmpDir, 'a', 'b'); + fs.mkdirSync(sub, { recursive: true }); + const dest = join(sub, 'file.cls'); + fs.writeFileSync(dest, 'content'); + + expect(findSymlinkOnPathSync(tmpDir, dest)).to.be.undefined; + }); + + it('should detect a symlinked file at the destination', () => { + const external = join(tmpDir, 'external.txt'); + fs.writeFileSync(external, 'external'); + const sub = join(tmpDir, 'project'); + fs.mkdirSync(sub); + const link = join(sub, 'link.txt'); + fs.symlinkSync(external, link); + + expect(findSymlinkOnPathSync(tmpDir, link)).to.equal(link); + }); + + it('should detect a symlinked ancestor directory', () => { + const externalDir = join(tmpDir, 'external'); + fs.mkdirSync(externalDir); + fs.writeFileSync(join(externalDir, 'victim.txt'), 'important data'); + const project = join(tmpDir, 'project'); + fs.mkdirSync(project); + const linkedDir = join(project, 'digitalExperiences'); + fs.symlinkSync(externalDir, linkedDir); + + const dest = join(linkedDir, 'victim.txt'); + expect(findSymlinkOnPathSync(tmpDir, dest)).to.equal(linkedDir); + }); + + it('should return undefined when path segments do not exist yet', () => { + const dest = join(tmpDir, 'nonexistent', 'deep', 'file.cls'); + expect(findSymlinkOnPathSync(tmpDir, dest)).to.be.undefined; + }); + + it('should reject destinations outside the root', () => { + const projectRoot = join(tmpDir, 'project'); + fs.mkdirSync(projectRoot); + const outside = join(tmpDir, 'outside', 'file.txt'); + + expect(findSymlinkOnPathSync(projectRoot, outside)).to.equal(outside); + }); + }); }); From 3e0cad1661c8c89f751df255f0e974360c0da43a Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Tue, 15 Sep 2026 08:22:59 -0600 Subject: [PATCH 5/8] test: add ancestor-symlink partial-delete scenario test Exercises the exact gap Eric identified: a DEB project where digitalExperiences/ is a symlink to an external directory. Verifies findSymlinkOnPathSync catches the ancestor symlink and the external file survives. --- test/client/retrieveExtract.test.ts | 33 +++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/client/retrieveExtract.test.ts b/test/client/retrieveExtract.test.ts index 8167bf7b5d..eff8d6b6af 100644 --- a/test/client/retrieveExtract.test.ts +++ b/test/client/retrieveExtract.test.ts @@ -21,6 +21,7 @@ import { XMLParser } from 'fast-xml-parser'; import { registry, RegistryAccess, SourceComponent, VirtualTreeContainer } from '../../src'; import { BotVersionFilter } from '../../src/client/types'; import { extractVersionNumber, filterAgentComponents, filterBotVersionEntries } from '../../src/client/retrieveExtract'; +import { findSymlinkOnPathSync } from '../../src/utils/fileSystemHandler'; describe('retrieveExtract - Version Filtering', () => { const registryAccess = new RegistryAccess(); @@ -932,4 +933,36 @@ describe('partial-delete symlink protection', () => { expect(fs.existsSync(symlinkPath)).to.be.false; expect(fs.existsSync(join(externalDir, 'important.txt'))).to.be.true; }); + + it('findSymlinkOnPathSync catches ancestor symlink in partial-delete scenario', () => { + // Simulate Eric's repro: digitalExperiences/ is a symlink to an external directory. + // A retrieve zip is missing one content file, so partial-delete would try to rmSync it. + // The ancestor-symlink check must prevent that deletion. + const packageRoot = join(tmpDir, 'force-app', 'main', 'default'); + const externalDir = join(tmpDir, 'outside-project'); + fs.mkdirSync(packageRoot, { recursive: true }); + fs.mkdirSync(join(externalDir, 'site', 'MySite1', 'sfdc_cms__view', 'home'), { recursive: true }); + + const victimFile = join(externalDir, 'site', 'MySite1', 'sfdc_cms__view', 'home', 'localOnly.json'); + fs.writeFileSync(victimFile, '{"local":"only content"}'); + + // Replace digitalExperiences with a symlink to the external directory + const deDir = join(packageRoot, 'digitalExperiences'); + fs.symlinkSync(externalDir, deDir); + + // This is the file path that handlePartialDeleteMerges would construct: + // packageRoot/digitalExperiences/site/MySite1/sfdc_cms__view/home/localOnly.json + const candidatePath = join(deDir, 'site', 'MySite1', 'sfdc_cms__view', 'home', 'localOnly.json'); + + // Verify the file is reachable through the symlink + expect(fs.existsSync(candidatePath)).to.be.true; + + // The ancestor-aware check must detect the symlinked digitalExperiences directory + const symlink = findSymlinkOnPathSync(packageRoot, candidatePath); + expect(symlink).to.equal(deDir); + + // Because findSymlinkOnPathSync returns truthy, deleteFilePath would skip the rmSync. + // Verify the external file survives. + expect(fs.existsSync(victimFile)).to.be.true; + }); }); From 6d2001a6d462e8fc8f873b7c3ea1cf04bf5a6e4c Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Tue, 15 Sep 2026 09:27:41 -0600 Subject: [PATCH 6/8] fix: warn when JWT access token and API version < 68 cause empty metadata results @W-24046915@ SOAP metadata operations silently fail when an org uses JWT-based access tokens and the API version is below 68.0. The ConnectionResolver catch block swallows these errors at debug level, producing a near-empty manifest with no user feedback. This adds a proactive warning before enumeration begins, telling users exactly what's wrong and how to fix it. --- messages/sdr.md | 4 +++ src/resolve/connectionResolver.ts | 18 ++++++++++ test/resolve/connectionResolver.test.ts | 47 ++++++++++++++++++++++++- 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/messages/sdr.md b/messages/sdr.md index 1db73bf5f7..084a553743 100644 --- a/messages/sdr.md +++ b/messages/sdr.md @@ -215,6 +215,10 @@ The directoryName '%s' for metadata type '%s' contains path segments that resolv The write path '%s' resolves outside the root destination '%s'. This may indicate a path traversal attempt via registryCustomizations. +# warning_jwt_api_version + +This org uses JWT-based access tokens, which require API version 68.0 or later for SOAP metadata operations. The current API version is %s, so metadata listing may return empty or incomplete results. To resolve, use --api-version 68 or update sourceApiVersion in sfdx-project.json. + # type_name_suggestions Confirm the metadata type name is correct. Validate against the registry at: diff --git a/src/resolve/connectionResolver.ts b/src/resolve/connectionResolver.ts index 885066d882..11e2b7b314 100644 --- a/src/resolve/connectionResolver.ts +++ b/src/resolve/connectionResolver.ts @@ -38,6 +38,18 @@ export type ResolveConnectionResult = { let requestCount = 0; let shouldQueryStandardValueSets = false; +function isJWTAccessToken(token: string | undefined | null): boolean { + if (!token) return false; + const parts = token.split('.'); + if (parts.length !== 3) return false; + try { + JSON.parse(Buffer.from(parts[0], 'base64url').toString()); + return true; + } catch { + return false; + } +} + let logger: Logger; const getLogger = (): Logger => { if (!logger) { @@ -89,6 +101,12 @@ export class ConnectionResolver { public async resolve( componentFilter = (component: Partial): boolean => isPlainObject(component) ): Promise { + if (isJWTAccessToken(this.connection.accessToken) && parseInt(this.connection.getApiVersion(), 10) < 68) { + void Lifecycle.getInstance().emitWarning( + messages.getMessage('warning_jwt_api_version', [this.connection.getApiVersion()]) + ); + } + // Aggregate array of metadata records in the org let aggregator: Array> = []; // Folder component type names. Each array value has the form [type::folder] diff --git a/test/resolve/connectionResolver.test.ts b/test/resolve/connectionResolver.test.ts index 78893454d6..d0be7d1a73 100644 --- a/test/resolve/connectionResolver.test.ts +++ b/test/resolve/connectionResolver.test.ts @@ -16,7 +16,7 @@ import { assert, expect, use } from 'chai'; import { MockTestOrgData, TestContext } from '@salesforce/core/testSetup'; -import { Connection } from '@salesforce/core'; +import { Connection, Lifecycle } from '@salesforce/core'; import { env } from '@salesforce/kit'; import deepEqualInAnyOrder from 'deep-equal-in-any-order'; import { ManageableState } from '../../src/client/types'; @@ -506,6 +506,51 @@ describe('ConnectionResolver', () => { }); }); + describe('JWT access token warning', () => { + const JWT_TOKEN = 'eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJ0ZXN0In0.signature'; + + it('should warn when connection has JWT token and API version below 68', async () => { + $$.SANDBOX.stub(connection.metadata, 'list'); + const emitWarningSpy = $$.SANDBOX.spy(Lifecycle.getInstance(), 'emitWarning'); + + connection.accessToken = JWT_TOKEN; + connection.setApiVersion('64.0'); + + const resolver = new ConnectionResolver(connection); + await resolver.resolve(); + + expect(emitWarningSpy.calledOnce).to.be.true; + expect(emitWarningSpy.firstCall.args[0]).to.include('JWT-based access tokens'); + expect(emitWarningSpy.firstCall.args[0]).to.include('64.0'); + }); + + it('should not warn when connection has JWT token and API version 68 or above', async () => { + $$.SANDBOX.stub(connection.metadata, 'list'); + const emitWarningSpy = $$.SANDBOX.spy(Lifecycle.getInstance(), 'emitWarning'); + + connection.accessToken = JWT_TOKEN; + connection.setApiVersion('68.0'); + + const resolver = new ConnectionResolver(connection); + await resolver.resolve(); + + expect(emitWarningSpy.called).to.be.false; + }); + + it('should not warn when connection has non-JWT token and API version below 68', async () => { + $$.SANDBOX.stub(connection.metadata, 'list'); + const emitWarningSpy = $$.SANDBOX.spy(Lifecycle.getInstance(), 'emitWarning'); + + connection.accessToken = '00D000000000000!AQcAQNotAJwtToken'; + connection.setApiVersion('64.0'); + + const resolver = new ConnectionResolver(connection); + await resolver.resolve(); + + expect(emitWarningSpy.called).to.be.false; + }); + }); + describe('missing filename and type', () => { it('should skip if component has undefined type and filename', async () => { const metadataQueryStub = $$.SANDBOX.stub(connection.metadata, 'list'); From 6a7b0b8248f35f5a2a0d66d5657a8ed5bfb52dab Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Tue, 15 Sep 2026 09:27:41 -0600 Subject: [PATCH 7/8] fix: warn when JWT access token and API version < 68 cause empty metadata results @W-24046915@ SOAP metadata operations silently fail when an org uses JWT-based access tokens and the API version is below 68.0. The ConnectionResolver catch block swallows these errors at debug level, producing a near-empty manifest with no user feedback. This adds a proactive warning before enumeration begins, telling users exactly what's wrong and how to fix it. --- messages/sdr.md | 4 +++ src/resolve/connectionResolver.ts | 18 ++++++++++ test/resolve/connectionResolver.test.ts | 47 ++++++++++++++++++++++++- 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/messages/sdr.md b/messages/sdr.md index ce7a98c481..64c4807bdf 100644 --- a/messages/sdr.md +++ b/messages/sdr.md @@ -211,6 +211,10 @@ The directoryName '%s' for metadata type '%s' contains path segments that resolv The write path '%s' resolves outside the root destination '%s'. This may indicate a path traversal attempt via registryCustomizations. +# warning_jwt_api_version + +This org uses JWT-based access tokens, which require API version 68.0 or later for SOAP metadata operations. The current API version is %s, so metadata listing may return empty or incomplete results. To resolve, use --api-version 68 or update sourceApiVersion in sfdx-project.json. + # type_name_suggestions Confirm the metadata type name is correct. Validate against the registry at: diff --git a/src/resolve/connectionResolver.ts b/src/resolve/connectionResolver.ts index 885066d882..11e2b7b314 100644 --- a/src/resolve/connectionResolver.ts +++ b/src/resolve/connectionResolver.ts @@ -38,6 +38,18 @@ export type ResolveConnectionResult = { let requestCount = 0; let shouldQueryStandardValueSets = false; +function isJWTAccessToken(token: string | undefined | null): boolean { + if (!token) return false; + const parts = token.split('.'); + if (parts.length !== 3) return false; + try { + JSON.parse(Buffer.from(parts[0], 'base64url').toString()); + return true; + } catch { + return false; + } +} + let logger: Logger; const getLogger = (): Logger => { if (!logger) { @@ -89,6 +101,12 @@ export class ConnectionResolver { public async resolve( componentFilter = (component: Partial): boolean => isPlainObject(component) ): Promise { + if (isJWTAccessToken(this.connection.accessToken) && parseInt(this.connection.getApiVersion(), 10) < 68) { + void Lifecycle.getInstance().emitWarning( + messages.getMessage('warning_jwt_api_version', [this.connection.getApiVersion()]) + ); + } + // Aggregate array of metadata records in the org let aggregator: Array> = []; // Folder component type names. Each array value has the form [type::folder] diff --git a/test/resolve/connectionResolver.test.ts b/test/resolve/connectionResolver.test.ts index 78893454d6..d0be7d1a73 100644 --- a/test/resolve/connectionResolver.test.ts +++ b/test/resolve/connectionResolver.test.ts @@ -16,7 +16,7 @@ import { assert, expect, use } from 'chai'; import { MockTestOrgData, TestContext } from '@salesforce/core/testSetup'; -import { Connection } from '@salesforce/core'; +import { Connection, Lifecycle } from '@salesforce/core'; import { env } from '@salesforce/kit'; import deepEqualInAnyOrder from 'deep-equal-in-any-order'; import { ManageableState } from '../../src/client/types'; @@ -506,6 +506,51 @@ describe('ConnectionResolver', () => { }); }); + describe('JWT access token warning', () => { + const JWT_TOKEN = 'eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJ0ZXN0In0.signature'; + + it('should warn when connection has JWT token and API version below 68', async () => { + $$.SANDBOX.stub(connection.metadata, 'list'); + const emitWarningSpy = $$.SANDBOX.spy(Lifecycle.getInstance(), 'emitWarning'); + + connection.accessToken = JWT_TOKEN; + connection.setApiVersion('64.0'); + + const resolver = new ConnectionResolver(connection); + await resolver.resolve(); + + expect(emitWarningSpy.calledOnce).to.be.true; + expect(emitWarningSpy.firstCall.args[0]).to.include('JWT-based access tokens'); + expect(emitWarningSpy.firstCall.args[0]).to.include('64.0'); + }); + + it('should not warn when connection has JWT token and API version 68 or above', async () => { + $$.SANDBOX.stub(connection.metadata, 'list'); + const emitWarningSpy = $$.SANDBOX.spy(Lifecycle.getInstance(), 'emitWarning'); + + connection.accessToken = JWT_TOKEN; + connection.setApiVersion('68.0'); + + const resolver = new ConnectionResolver(connection); + await resolver.resolve(); + + expect(emitWarningSpy.called).to.be.false; + }); + + it('should not warn when connection has non-JWT token and API version below 68', async () => { + $$.SANDBOX.stub(connection.metadata, 'list'); + const emitWarningSpy = $$.SANDBOX.spy(Lifecycle.getInstance(), 'emitWarning'); + + connection.accessToken = '00D000000000000!AQcAQNotAJwtToken'; + connection.setApiVersion('64.0'); + + const resolver = new ConnectionResolver(connection); + await resolver.resolve(); + + expect(emitWarningSpy.called).to.be.false; + }); + }); + describe('missing filename and type', () => { it('should skip if component has undefined type and filename', async () => { const metadataQueryStub = $$.SANDBOX.stub(connection.metadata, 'list'); From e85bcdb6f4782be37a7ee062a3b1b7be00af1ebb Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Wed, 16 Sep 2026 13:35:25 -0600 Subject: [PATCH 8/8] docs: fix api version --- messages/sdr.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/messages/sdr.md b/messages/sdr.md index 084a553743..559ae04337 100644 --- a/messages/sdr.md +++ b/messages/sdr.md @@ -217,7 +217,7 @@ The write path '%s' resolves outside the root destination '%s'. This may indicat # warning_jwt_api_version -This org uses JWT-based access tokens, which require API version 68.0 or later for SOAP metadata operations. The current API version is %s, so metadata listing may return empty or incomplete results. To resolve, use --api-version 68 or update sourceApiVersion in sfdx-project.json. +This org uses JWT-based access tokens, which require API version 68.0 or later for SOAP metadata operations. The current API version is %s, so metadata listing may return empty or incomplete results. To resolve, update sourceApiVersion in sfdx-project.json to 68.0 or higher. # type_name_suggestions