diff --git a/.changeset/lucky-countries-smile.md b/.changeset/lucky-countries-smile.md new file mode 100644 index 0000000000..c9066f109f --- /dev/null +++ b/.changeset/lucky-countries-smile.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Use `resolveSafeChildPath` in the `fetchContents` function to forbid reading files outside the base directory when a template is registered from a `file:` location. diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/helpers.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/helpers.test.ts index 7c7719cf68..a738afbe3f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/helpers.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/helpers.test.ts @@ -67,7 +67,19 @@ describe('fetchContent helper', () => { fetchUrl: '/etc/passwd', }), ).rejects.toThrow( - 'Fetch URL may not be absolute for file locations, /etc/passwd', + 'Relative path is not allowed to refer to a directory outside its parent', + ); + }); + + it('should reject relative file locations that exit the baseUrl', async () => { + await expect( + fetchContents({ + ...options, + baseUrl: 'file:///some/path', + fetchUrl: '../test', + }), + ).rejects.toThrow( + 'Relative path is not allowed to refer to a directory outside its parent', ); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/helpers.ts index 390ab7ec39..5625989417 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/helpers.ts @@ -14,12 +14,12 @@ * limitations under the License. */ -import fs from 'fs-extra'; -import { resolve as resolvePath, isAbsolute } from 'path'; -import { UrlReader } from '@backstage/backend-common'; +import { resolveSafeChildPath, UrlReader } from '@backstage/backend-common'; +import { JsonValue } from '@backstage/config'; import { InputError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; -import { JsonValue } from '@backstage/config'; +import fs from 'fs-extra'; +import * as path from 'path'; export async function fetchContents({ reader, @@ -52,12 +52,7 @@ export async function fetchContents({ // We handle both file locations and url ones if (!fetchUrlIsAbsolute && baseUrl?.startsWith('file://')) { const basePath = baseUrl.slice('file://'.length); - if (isAbsolute(fetchUrl)) { - throw new InputError( - `Fetch URL may not be absolute for file locations, ${fetchUrl}`, - ); - } - const srcDir = resolvePath(basePath, '..', fetchUrl); + const srcDir = resolveSafeChildPath(path.dirname(basePath), fetchUrl); await fs.copy(srcDir, outputPath); } else { let readUrl;