-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(functions): add remoteSource utility and path resolution helper #9077
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
111dd0e
feat(functions): add remoteSource utility and path resolution helper
taeold 5ecd08a
refactor: apply gemini suggestions.
taeold 920b86f
refactor: simplify git clone logic.
taeold ee5ed88
style: formatter
taeold ffbb5b7
Merge branch 'master' into pr-remote-src-pr2-master
taeold 1d4a387
nit: simplify impl.
taeold f5fb8e2
fix: add missing return value.
taeold 4576da0
Merge branch 'master' into pr-remote-src-pr2-master
taeold 7364d00
Merge branch 'master' into pr-remote-src-pr2-master
taeold 1ea6a1d
Merge branch 'master' into pr-remote-src-pr2-master
taeold 91963ad
use GitHub archive API instead.
taeold 5d53070
Update src/deploy/functions/remoteSource.ts
taeold 9fb6ee1
Merge branch 'master' into pr-remote-src-pr2-master
taeold b94e744
refactor: Remove `pathUtils` module by adding `resolveWithin` to `uti…
taeold c871f88
Merge branch 'pr-remote-src-pr2-master' of https://github.com/firebas…
taeold 266b424
refactor: improve remote source download and add functions.yaml valid…
taeold cc83bdd
nit: make formatter happy
taeold ec8f2fa
Merge branch 'master' into pr-remote-src-pr2-master
taeold a343c9e
nit: update misleading comment
taeold 5fbbb73
nit: nit
taeold d7ee637
respond to pr comments.
taeold a6dd70f
nit: run formatter
taeold f23ee86
Merge branch 'master' into pr-remote-src-pr2-master
taeold File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,246 @@ | ||
| import { expect } from "chai"; | ||
| import * as sinon from "sinon"; | ||
| import * as fs from "fs"; | ||
| import * as path from "path"; | ||
| import * as mockfs from "mock-fs"; | ||
| import * as archiver from "archiver"; | ||
| import { Writable } from "stream"; | ||
|
|
||
| import { getRemoteSource, requireFunctionsYaml } from "./remoteSource"; | ||
| import { FirebaseError } from "../../error"; | ||
| import * as downloadUtils from "../../downloadUtils"; | ||
|
|
||
| describe("remoteSource", () => { | ||
| describe("requireFunctionsYaml", () => { | ||
| afterEach(() => { | ||
| mockfs.restore(); | ||
| }); | ||
|
|
||
| it("should not throw if functions.yaml exists", () => { | ||
| mockfs({ | ||
| "/app/functions.yaml": "runtime: nodejs22", | ||
| }); | ||
|
|
||
| expect(() => requireFunctionsYaml("/app")).to.not.throw(); | ||
| }); | ||
|
|
||
| it("should throw FirebaseError if functions.yaml is missing", () => { | ||
| mockfs({ | ||
| "/app/index.js": "console.log('hello')", | ||
| }); | ||
|
|
||
| expect(() => requireFunctionsYaml("/app")).to.throw( | ||
| FirebaseError, | ||
| /The remote repository is missing a required deployment manifest/, | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe("getRemoteSource", () => { | ||
| let downloadToTmpStub: sinon.SinonStub; | ||
|
|
||
| beforeEach(() => { | ||
| downloadToTmpStub = sinon.stub(downloadUtils, "downloadToTmp"); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| sinon.restore(); | ||
| mockfs.restore(); | ||
| }); | ||
|
|
||
| async function createZipBuffer( | ||
| files: { [path: string]: string }, | ||
| topLevelDir?: string, | ||
| ): Promise<Buffer> { | ||
| const archive = archiver("zip", { zlib: { level: 9 } }); | ||
| const chunks: Buffer[] = []; | ||
| const output = new Writable({ | ||
| write(chunk, _encoding, callback) { | ||
| chunks.push(chunk instanceof Buffer ? chunk : Buffer.from(chunk)); | ||
| callback(); | ||
| }, | ||
| }); | ||
|
|
||
| return new Promise((resolve, reject) => { | ||
| output.on("finish", () => resolve(Buffer.concat(chunks as unknown as Uint8Array[]))); | ||
| archive.on("error", (err) => reject(err)); | ||
| archive.pipe(output); | ||
|
|
||
| for (const [filePath, content] of Object.entries(files)) { | ||
| const entryPath = topLevelDir ? path.join(topLevelDir, filePath) : filePath; | ||
| archive.append(content, { name: entryPath }); | ||
| } | ||
| archive.finalize(); | ||
| }); | ||
| } | ||
|
|
||
| it("should use GitHub Archive API for GitHub URLs", async () => { | ||
| const zipBuffer = await createZipBuffer( | ||
| { "functions.yaml": "runtime: nodejs22" }, | ||
| "repo-main", | ||
| ); | ||
| mockfs({ | ||
| "/tmp/source.zip": zipBuffer, | ||
| "/dest": {}, | ||
| }); | ||
| downloadToTmpStub.resolves("/tmp/source.zip"); | ||
|
|
||
| const sourceDir = await getRemoteSource("https://github.com/org/repo", "main", "/dest"); | ||
|
|
||
| expect(downloadToTmpStub.calledOnce).to.be.true; | ||
| expect(downloadToTmpStub.firstCall.args[0]).to.equal( | ||
| "https://github.com/org/repo/archive/main.zip", | ||
| ); | ||
| expect(sourceDir).to.match(/repo-main$/); | ||
| expect(sourceDir).to.contain("/dest"); | ||
| expect(fs.statSync(path.join(sourceDir, "functions.yaml")).isFile()).to.be.true; | ||
| }); | ||
|
|
||
| it("should support org/repo shorthand", async () => { | ||
| const zipBuffer = await createZipBuffer( | ||
| { "functions.yaml": "runtime: nodejs22" }, | ||
| "repo-main", | ||
| ); | ||
| mockfs({ | ||
| "/tmp/source.zip": zipBuffer, | ||
| "/dest": {}, | ||
| }); | ||
| downloadToTmpStub.resolves("/tmp/source.zip"); | ||
|
|
||
| const sourceDir = await getRemoteSource("org/repo", "main", "/dest"); | ||
|
|
||
| expect(downloadToTmpStub.calledOnce).to.be.true; | ||
| expect(downloadToTmpStub.firstCall.args[0]).to.equal( | ||
| "https://github.com/org/repo/archive/main.zip", | ||
| ); | ||
| expect(sourceDir).to.match(/repo-main$/); | ||
| }); | ||
|
|
||
| it("should strip top-level directory from GitHub archive", async () => { | ||
| const zipBuffer = await createZipBuffer( | ||
| { "functions.yaml": "runtime: nodejs22" }, | ||
| "repo-main", | ||
| ); | ||
| mockfs({ | ||
| "/tmp/source.zip": zipBuffer, | ||
| "/dest": {}, | ||
| }); | ||
| downloadToTmpStub.resolves("/tmp/source.zip"); | ||
|
|
||
| const sourceDir = await getRemoteSource("https://github.com/org/repo", "main", "/dest"); | ||
|
|
||
| expect(sourceDir).to.match(/repo-main$/); | ||
| expect(fs.statSync(path.join(sourceDir, "functions.yaml")).isFile()).to.be.true; | ||
| }); | ||
|
|
||
| it("should NOT strip top-level directory if multiple files exist at root", async () => { | ||
| const zipBuffer = await createZipBuffer({ | ||
| "file1.txt": "content", | ||
| "functions.yaml": "runtime: nodejs22", | ||
| "repo-main/index.js": "console.log('hello')", | ||
| }); | ||
| mockfs({ | ||
| "/tmp/source.zip": zipBuffer, | ||
| "/dest": {}, | ||
| }); | ||
| downloadToTmpStub.resolves("/tmp/source.zip"); | ||
|
|
||
| const sourceDir = await getRemoteSource("https://github.com/org/repo", "main", "/dest"); | ||
|
|
||
| expect(sourceDir).to.not.match(/repo-main$/); | ||
| expect(sourceDir).to.equal("/dest"); | ||
| expect(fs.statSync(path.join(sourceDir, "file1.txt")).isFile()).to.be.true; | ||
| expect(fs.statSync(path.join(sourceDir, "functions.yaml")).isFile()).to.be.true; | ||
| }); | ||
|
|
||
| it("should throw error if GitHub Archive download fails", async () => { | ||
| mockfs({ "/dest": {} }); | ||
| downloadToTmpStub.rejects(new Error("404 Not Found")); | ||
|
|
||
| await expect( | ||
| getRemoteSource("https://github.com/org/repo", "main", "/dest"), | ||
| ).to.be.rejectedWith(FirebaseError, /Failed to download GitHub archive/); | ||
| }); | ||
|
|
||
| it("should throw error for non-GitHub URLs", async () => { | ||
| mockfs({ "/dest": {} }); | ||
| await expect( | ||
| getRemoteSource("https://gitlab.com/org/repo", "main", "/dest"), | ||
| ).to.be.rejectedWith(FirebaseError, /Only GitHub repositories are supported/); | ||
| }); | ||
|
|
||
| it("should validate subdirectory exists after clone", async () => { | ||
| const zipBuffer = await createZipBuffer( | ||
| { "functions.yaml": "runtime: nodejs22" }, | ||
| "repo-main", | ||
| ); | ||
| mockfs({ | ||
| "/tmp/source.zip": zipBuffer, | ||
| "/dest": {}, | ||
| }); | ||
| downloadToTmpStub.resolves("/tmp/source.zip"); | ||
|
|
||
| await expect( | ||
| getRemoteSource("https://github.com/org/repo", "main", "/dest", "nonexistent"), | ||
| ).to.be.rejectedWith(FirebaseError, /Directory 'nonexistent' not found/); | ||
| }); | ||
|
|
||
| it("should return source even if functions.yaml is missing", async () => { | ||
| const zipBuffer = await createZipBuffer({ "index.js": "console.log('hello')" }, "repo-main"); | ||
| mockfs({ | ||
| "/tmp/source.zip": zipBuffer, | ||
| "/dest": {}, | ||
| }); | ||
| downloadToTmpStub.resolves("/tmp/source.zip"); | ||
|
|
||
| const sourceDir = await getRemoteSource("https://github.com/org/repo", "main", "/dest"); | ||
|
|
||
| expect(sourceDir).to.match(/repo-main$/); | ||
| expect(fs.statSync(path.join(sourceDir, "index.js")).isFile()).to.be.true; | ||
| expect(() => fs.statSync(path.join(sourceDir, "functions.yaml"))).to.throw(); | ||
| }); | ||
|
|
||
| it("should prevent path traversal in subdirectory", async () => { | ||
| const zipBuffer = await createZipBuffer( | ||
| { "functions.yaml": "runtime: nodejs22" }, | ||
| "repo-main", | ||
| ); | ||
| mockfs({ | ||
| "/tmp/source.zip": zipBuffer, | ||
| "/dest": {}, | ||
| }); | ||
| downloadToTmpStub.resolves("/tmp/source.zip"); | ||
|
|
||
| await expect( | ||
| getRemoteSource("https://github.com/org/repo", "main", "/dest", "../outside"), | ||
| ).to.be.rejectedWith(FirebaseError, /must not escape/); | ||
| }); | ||
|
|
||
| it("should return subdirectory if specified", async () => { | ||
| const zipBuffer = await createZipBuffer( | ||
| { | ||
| "functions.yaml": "runtime: nodejs22", | ||
| "app/index.js": "console.log('hello')", | ||
| "app/functions.yaml": "runtime: nodejs22", | ||
| }, | ||
| "repo-main", | ||
| ); | ||
| mockfs({ | ||
| "/tmp/source.zip": zipBuffer, | ||
| "/dest": {}, | ||
| }); | ||
| downloadToTmpStub.resolves("/tmp/source.zip"); | ||
|
|
||
| const sourceDir = await getRemoteSource( | ||
| "https://github.com/org/repo", | ||
| "main", | ||
| "/dest", | ||
| "app", | ||
| ); | ||
|
|
||
| expect(sourceDir).to.match(/repo-main\/app$/); | ||
| expect(fs.statSync(path.join(sourceDir, "index.js")).isFile()).to.be.true; | ||
| expect(fs.statSync(path.join(sourceDir, "functions.yaml")).isFile()).to.be.true; | ||
| }); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| import * as fs from "fs"; | ||
| import * as path from "path"; | ||
|
|
||
| import { URL } from "url"; | ||
|
|
||
| import { FirebaseError } from "../../error"; | ||
| import { logger } from "../../logger"; | ||
| import { logLabeledBullet, resolveWithin } from "../../utils"; | ||
| import { dirExistsSync, fileExistsSync } from "../../fsutils"; | ||
| import * as downloadUtils from "../../downloadUtils"; | ||
| import * as unzipModule from "../../unzip"; | ||
|
|
||
| /** | ||
| * Downloads a remote source to a temporary directory and returns the absolute path | ||
| * to the source directory. | ||
| * | ||
| * @param repository Remote URL (e.g. https://github.com/org/repo) or shorthand (org/repo) | ||
| * @param ref Git ref to fetch (tag/branch/commit) | ||
| * @param destDir Directory to extract the source code to | ||
| * @param subDir Optional subdirectory within the repo to use | ||
| * @return Absolute path to the checked‑out source directory | ||
| */ | ||
| export async function getRemoteSource( | ||
| repository: string, | ||
| ref: string, | ||
| destDir: string, | ||
| subDir?: string, | ||
| ): Promise<string> { | ||
| logger.debug( | ||
| `Downloading remote source: ${repository}@${ref} (destDir: ${destDir}, subDir: ${subDir || "."})`, | ||
| ); | ||
|
|
||
| const gitHubInfo = parseGitHubUrl(repository); | ||
| if (!gitHubInfo) { | ||
| throw new FirebaseError( | ||
| `Could not parse GitHub repository URL: ${repository}. ` + | ||
| `Only GitHub repositories are supported.`, | ||
| ); | ||
| } | ||
|
|
||
| let rootDir = destDir; | ||
| try { | ||
| logger.debug(`Attempting to download via GitHub Archive API for ${repository}@${ref}...`); | ||
| const archiveUrl = `https://github.com/${gitHubInfo.owner}/${gitHubInfo.repo}/archive/${ref}.zip`; | ||
| const archivePath = await downloadUtils.downloadToTmp(archiveUrl); | ||
| logger.debug(`Downloaded archive to ${archivePath}, unzipping...`); | ||
|
|
||
| await unzipModule.unzip(archivePath, destDir); | ||
|
|
||
| // GitHub archives usually wrap content in a top-level directory (e.g. repo-ref). | ||
| // We need to find it and use it as the root. | ||
| const files = fs.readdirSync(destDir); | ||
|
|
||
| if (files.length === 1 && fs.statSync(path.join(destDir, files[0])).isDirectory()) { | ||
| rootDir = path.join(destDir, files[0]); | ||
| logger.debug(`Found top-level directory in archive: ${files[0]}`); | ||
| } | ||
| } catch (err: unknown) { | ||
| throw new FirebaseError( | ||
| `Failed to download GitHub archive for ${repository}@${ref}. ` + | ||
| `Make sure the repository is public and the ref exists. ` + | ||
| `Private repositories are not supported via this method.`, | ||
| { original: err as Error }, | ||
| ); | ||
| } | ||
|
|
||
| const sourceDir = subDir | ||
| ? resolveWithin( | ||
| rootDir, | ||
| subDir, | ||
| `Subdirectory '${subDir}' in remote source must not escape the repository root.`, | ||
| ) | ||
| : rootDir; | ||
|
|
||
| if (subDir && !dirExistsSync(sourceDir)) { | ||
| throw new FirebaseError(`Directory '${subDir}' not found in repository ${repository}@${ref}`); | ||
| } | ||
|
|
||
taeold marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| const origin = `${repository}@${ref}${subDir ? `/${subDir}` : ""}`; | ||
taeold marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| logLabeledBullet("functions", `downloaded remote source (${origin})`); | ||
| return sourceDir; | ||
| } | ||
|
|
||
| /** | ||
| * Parses a GitHub repository URL or shorthand string into its owner and repo components. | ||
| * | ||
| * Valid inputs include: | ||
| * - "https://github.com/owner/repo" | ||
| * - "https://github.com/owner/repo.git" | ||
| * - "owner/repo" | ||
| * @param url The URL or shorthand string to parse. | ||
| * @return An object containing the owner and repo, or undefined if parsing fails. | ||
| */ | ||
| function parseGitHubUrl(url: string): { owner: string; repo: string } | undefined { | ||
| // Handle "org/repo" shorthand | ||
| const shorthandMatch = /^[a-zA-Z0-9-]+\/[a-zA-Z0-9-_.]+$/.exec(url); | ||
| if (shorthandMatch) { | ||
| const [owner, repo] = url.split("/"); | ||
| return { owner, repo }; | ||
| } | ||
|
|
||
| try { | ||
| const u = new URL(url); | ||
| if (u.hostname !== "github.com") { | ||
| return undefined; | ||
| } | ||
| const parts = u.pathname.split("/").filter((p) => !!p); | ||
| if (parts.length < 2) { | ||
| return undefined; | ||
| } | ||
| const owner = parts[0]; | ||
| let repo = parts[1]; | ||
| if (repo.endsWith(".git")) { | ||
| repo = repo.slice(0, -4); | ||
| } | ||
| return { owner, repo }; | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Verifies that a `functions.yaml` manifest exists at the given directory. | ||
| * Throws a FirebaseError with guidance if it is missing. | ||
| */ | ||
| export function requireFunctionsYaml(codeDir: string): void { | ||
| const functionsYamlPath = path.join(codeDir, "functions.yaml"); | ||
| if (!fileExistsSync(functionsYamlPath)) { | ||
| throw new FirebaseError( | ||
| `The remote repository is missing a required deployment manifest (functions.yaml).\n\n` + | ||
| `For your security, Firebase requires a static manifest to deploy functions from a remote source. ` + | ||
| `This prevents the execution of arbitrary code on your machine during the function discovery process.\n\n` + | ||
| `If you trust this repository and want to use it anyway, clone the repository locally, inspect the code for safety, and deploy it as a local source.`, | ||
| ); | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.