From 9e2bbcd46f946e967d459bcfe4c8bfedf076004c Mon Sep 17 00:00:00 2001 From: herdiyanitdev <82978131+herdiyana256@users.noreply.github.com> Date: Thu, 13 Aug 2026 22:56:51 +0700 Subject: [PATCH 01/11] fix(deploy): prevent command injection from angular.json in ng deploy (#3738) The SSR deploy builder shelled out with values read straight from angular.json: the package manager and each server externalDependencies entry reached `execSync` as a command string, as did the functions output path. A malicious or cloned workspace could run arbitrary commands the moment a developer ran `ng deploy`. Both calls now go through a single runner built on cross-spawn, which passes arguments as argv entries and launches the Windows .cmd shims that child_process.execFile cannot, so the deploy keeps working cross-platform without a shell. The package manager is checked against a supported set and dependency names are rejected unless they are plain specifiers. Tests cover the validators and assert the call sites route through the shell-free runner, so reverting to a shell or dropping a validator fails the suite. (cherry picked from commit 0e8ed0cb130b227118180190056a00990db2ee1e) --- package-lock.json | 11 +++ package.json | 1 + src/schematics/deploy/actions.jasmine.ts | 90 +++++++++++++++++++++++- src/schematics/deploy/actions.ts | 84 ++++++++++++++++++++-- 4 files changed, 181 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 29cf309fc..e6e6ebc02 100644 --- a/package-lock.json +++ b/package-lock.json @@ -46,6 +46,7 @@ "@angular/cli": "^20.0.0", "@angular/compiler-cli": "^20.0.0", "@angular/platform-server": "^20.0.0", + "@types/cross-spawn": "^6.0.6", "@types/fs-extra": "^7.0.0", "@types/gzip-size": "^5.1.1", "@types/inquirer": "^0.0.44", @@ -7584,6 +7585,16 @@ "@types/node": "*" } }, + "node_modules/@types/cross-spawn": { + "version": "6.0.6", + "resolved": "https://registry.npmjs.org/@types/cross-spawn/-/cross-spawn-6.0.6.tgz", + "integrity": "sha512-fXRhhUkG4H3TQk5dBhQ7m/JDdSNHKwR2BBia62lhwEIq9xGiQKLxd6LymNhn47SjXhsUEPmxi+PKw2OkW4LLjA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@types/eslint": { "version": "9.6.1", "resolved": "https://registry.npmjs.org/@types/eslint/-/eslint-9.6.1.tgz", diff --git a/package.json b/package.json index 833750ce3..012abc998 100644 --- a/package.json +++ b/package.json @@ -87,6 +87,7 @@ "@angular/cli": "^20.0.0", "@angular/compiler-cli": "^20.0.0", "@angular/platform-server": "^20.0.0", + "@types/cross-spawn": "^6.0.6", "@types/fs-extra": "^7.0.0", "@types/gzip-size": "^5.1.1", "@types/inquirer": "^0.0.44", diff --git a/src/schematics/deploy/actions.jasmine.ts b/src/schematics/deploy/actions.jasmine.ts index abae2e375..2221421db 100644 --- a/src/schematics/deploy/actions.jasmine.ts +++ b/src/schematics/deploy/actions.jasmine.ts @@ -3,7 +3,7 @@ import { join } from 'path'; import { BuilderContext, BuilderRun, ScheduleOptions, Target } from '@angular-devkit/architect'; import { JsonObject, logging } from '@angular-devkit/core'; import { BuildTarget, FSHost, FirebaseDeployConfig, FirebaseTools } from '../interfaces'; -import deploy, { deployToFunction } from './actions' +import deploy, { assertSafeDependencyName, assertSupportedPackageManager, deployToFunction, findPackageVersion, processHost } from './actions' import 'jasmine'; let context: BuilderContext; @@ -300,3 +300,91 @@ describe('universal deployment', () => { expect(spy).not.toHaveBeenCalled(); });*/ }); + +describe('deploy input validation (command-injection hardening)', () => { + describe('assertSupportedPackageManager', () => { + ['npm', 'yarn', 'pnpm', 'cnpm', 'bun'].forEach((pm) => { + it(`allows the supported package manager "${pm}"`, () => { + expect(assertSupportedPackageManager(pm)).toBe(pm); + }); + }); + + it('rejects a package manager carrying a shell payload', () => { + expect(() => assertSupportedPackageManager('npm; touch /tmp/pwned #')) + .toThrowError(/Unsupported package manager/); + }); + + it('rejects an arbitrary executable path', () => { + expect(() => assertSupportedPackageManager('/tmp/evil')).toThrowError(/Unsupported package manager/); + }); + }); + + describe('assertSafeDependencyName', () => { + ['rxjs', '@angular/core', '@angular/*', 'some-pkg', 'a.b_c'].forEach((name) => { + it(`allows the valid dependency name "${name}"`, () => { + expect(assertSafeDependencyName(name)).toBe(name); + }); + }); + + ['evil; touch /tmp/pwned #', 'a b', '$(id)', '`id`', 'a|b', 'a&b', '-rf', '', 'a>b'].forEach((name) => { + it(`rejects the unsafe dependency name ${JSON.stringify(name)}`, () => { + expect(() => assertSafeDependencyName(name)).toThrowError(/Invalid dependency name/); + }); + }); + }); + + // These guard the fix at its call sites: the validators above are only useful + // if the deploy code keeps routing every command through the shell-free runner. + // A regression to execSync/`shell: true`, or a dropped validator call, fails here. + describe('call sites route through the shell-free runner', () => { + beforeEach(() => initMocks()); + + it('installs functions dependencies via the runner with an argv array and no shell', async () => { + // The install branch only runs when the generated package.json exists. + spyOn(fsHost, 'existsSync').and.returnValue(true); + const runSpy = spyOn(processHost, 'runPackageBin').and.returnValue(Buffer.from('')); + + await deployToFunction( + firebaseMock, + context, + workspaceRoot, + STATIC_BUILD_TARGET, + SERVER_BUILD_TARGET, + { preview: false }, + undefined, + fsHost + ); + + expect(runSpy).toHaveBeenCalledTimes(1); + const [command, args, options] = runSpy.calls.mostRecent().args; + expect(command).toBe('npm'); + expect(args).toEqual(['--prefix', join(workspaceRoot, 'dist'), 'install']); + // No shell: a `shell` option would reopen the injection this PR closes. + expect((options as any)?.shell).toBeFalsy(); + }); + + it('runs the package manager through the runner with a validated argv array', () => { + const runSpy = spyOn(processHost, 'runPackageBin').and.returnValue(Buffer.from('')); + + findPackageVersion('npm', 'rxjs'); + + expect(runSpy).toHaveBeenCalledWith('npm', ['list', 'rxjs']); + }); + + it('rejects an unsupported package manager before spawning anything', () => { + const runSpy = spyOn(processHost, 'runPackageBin'); + + expect(() => findPackageVersion('npm; touch /tmp/pwned #', 'rxjs')) + .toThrowError(/Unsupported package manager/); + expect(runSpy).not.toHaveBeenCalled(); + }); + + it('rejects an unsafe dependency name before spawning anything', () => { + const runSpy = spyOn(processHost, 'runPackageBin'); + + expect(() => findPackageVersion('npm', 'evil; touch /tmp/pwned #')) + .toThrowError(/Invalid dependency name/); + expect(runSpy).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/schematics/deploy/actions.ts b/src/schematics/deploy/actions.ts index a4556124a..124aef857 100644 --- a/src/schematics/deploy/actions.ts +++ b/src/schematics/deploy/actions.ts @@ -1,8 +1,9 @@ -import { SpawnOptionsWithoutStdio, execSync, spawn } from 'child_process'; +import { SpawnOptionsWithoutStdio, SpawnSyncOptions, spawn } from 'child_process'; import { existsSync, readFileSync, renameSync, writeFileSync } from 'fs'; import { dirname, join } from 'path'; import { BuilderContext, targetFromTargetString } from '@angular-devkit/architect'; import { SchematicsException } from '@angular-devkit/schematics'; +import crossSpawn from 'cross-spawn'; import { copySync, removeSync } from 'fs-extra'; import * as inquirer from 'inquirer'; import open from 'open'; @@ -115,8 +116,78 @@ const defaultFsHost: FSHost = { existsSync, }; -const findPackageVersion = (packageManager: string, name: string) => { - const match = execSync(`${packageManager} list ${name}`).toString().match(`[^|s]${escapeRegExp(name)}[@| ][^s]+(s.+)?$`); +// Package managers AngularFire is willing to shell out to when resolving +// dependency versions. Kept in sync with the Angular CLI's own list. The value +// comes from `cli.packageManager` in angular.json, which this builder reads with +// a raw `JSON.parse`, i.e. it is NOT run through the Angular CLI's own schema +// validation, so it must be checked here before it is ever used as an argv0. +export const SUPPORTED_PACKAGE_MANAGERS = ['npm', 'yarn', 'pnpm', 'cnpm', 'bun']; + +export const assertSupportedPackageManager = (packageManager: string): string => { + if (!SUPPORTED_PACKAGE_MANAGERS.includes(packageManager)) { + throw new SchematicsException( + `Unsupported package manager "${packageManager}" in angular.json (cli.packageManager). ` + + `Expected one of: ${SUPPORTED_PACKAGE_MANAGERS.join(', ')}.` + ); + } + return packageManager; +}; + +// A dependency name comes from `architect..server.options.externalDependencies` +// in angular.json. Reject anything that is not a plain package specifier so it can +// neither inject shell metacharacters (defence in depth alongside execFileSync) nor +// be parsed as a CLI flag by the package manager (argument injection). +export const assertSafeDependencyName = (name: string): string => { + // Valid npm package names / esbuild external globs never contain whitespace or + // shell metacharacters, and never start with a dash. Reject anything else so the + // value can neither inject a shell command (defence in depth alongside + // execFileSync) nor be parsed as a package-manager flag (argument injection). + if (typeof name !== 'string' || name.length === 0 || name.startsWith('-') || + /[\s;&|$`(){}<>!\\'"]/.test(name)) { + throw new SchematicsException( + `Invalid dependency name ${JSON.stringify(name)} in angular.json (server externalDependencies).` + ); + } + return name; +}; + +// All shelling out from the deploy builder funnels through this single runner. +// cross-spawn (v7) resolves the platform-appropriate executable and escapes each +// argument, so a value taken from angular.json is passed as an argv entry and can +// never be parsed as shell syntax. Unlike child_process.execFile it can launch a +// Windows `.cmd`/`.bat` shim (npm, yarn, pnpm and cnpm all ship as `.cmd` shims +// there), so `ng deploy` keeps working cross-platform. We deliberately avoid +// `shell: true`, whose args-array form Node runtime-deprecates (DEP0190) because +// it concatenates the arguments without escaping, reopening the injection. +// It is exported as an object so tests can assert the deploy code shells out only +// through here and never regresses to execSync or `shell: true`. +export const processHost = { + runPackageBin( + command: string, + args: string[], + options: SpawnSyncOptions = {}, + ): Buffer { + const result = crossSpawn.sync(command, args, options); + if (result.error) { throw result.error; } + if (result.status !== 0) { + throw new SchematicsException( + `Command "${command}" exited with ${result.signal ? `signal ${result.signal}` : `code ${result.status}`}.` + ); + } + return result.stdout; + }, +}; + +export const findPackageVersion = (packageManager: string, name: string) => { + // Run the package manager without a shell (argument array, no `shell` option) + // so a dependency name or package-manager value taken from angular.json cannot + // be interpreted as a shell command. Both values are validated first, so an + // unsupported manager or unsafe name throws before anything is ever spawned. + const output = processHost.runPackageBin(assertSupportedPackageManager(packageManager), [ + 'list', + assertSafeDependencyName(name), + ]).toString(); + const match = output.match(`[^|s]${escapeRegExp(name)}[@| ][^s]+(s.+)?$`); return match ? match[0].split(new RegExp(`${escapeRegExp(name)}[@| ]`))[1].split(/\s/)[0] : null; }; @@ -238,7 +309,12 @@ export const deployToFunction = async ( const siteTarget = options.target ?? context.target!.project; if (fsHost.existsSync(functionsPackageJsonPath)) { - execSync(`npm --prefix ${functionsOut} install`); + // Pass the output directory as an argv entry rather than interpolating it + // into a shell string; `functionsOut` derives from the `outputPath` deploy + // option (angular.json) and must not be able to inject shell commands. npm is + // a `.cmd` shim on Windows, which child_process.execFile cannot launch, so + // this goes through the shell-free cross-spawn runner instead. + processHost.runPackageBin('npm', ['--prefix', functionsOut, 'install'], { stdio: 'inherit' }); } else { console.error(`No package.json exists at ${functionsOut}`); } From 706a712007e1a8378fbe82276d1eda1a2aa67b18 Mon Sep 17 00:00:00 2001 From: herdiyanitdev <82978131+herdiyana256@users.noreply.github.com> Date: Tue, 18 Aug 2026 00:26:22 +0700 Subject: [PATCH 02/11] fix(deploy): pass gcloud arguments as an array instead of a joined string (#3726) spawnAsync built each gcloud command as one string and split it on whitespace before calling spawn(), so any deploy option containing a space was chopped into extra argv entries and a value from angular.json could add flags of its own. It now takes command and args separately and the three call sites pass arrays, removing the join and split entirely. The close handler also rejected only on exit code 1, so a gcloud failure with any other code, or a process killed by a signal, resolved as success and a failed deploy reported as done. It now rejects on any non-zero or null code. The Cloud Run argument construction moved into two exported functions so tests can assert the argv shape without mocking spawn, and functionName and region gained schema patterns, since both also reach the generated Cloud Functions source. (cherry picked from commit 778625adc71186d3de33c11d10994052d93e1dff) --- src/schematics/deploy/actions.jasmine.ts | 45 ++++++++++++++++++- src/schematics/deploy/actions.ts | 56 +++++++++++++++++------- src/schematics/deploy/schema.json | 6 ++- 3 files changed, 88 insertions(+), 19 deletions(-) diff --git a/src/schematics/deploy/actions.jasmine.ts b/src/schematics/deploy/actions.jasmine.ts index 2221421db..b34603b71 100644 --- a/src/schematics/deploy/actions.jasmine.ts +++ b/src/schematics/deploy/actions.jasmine.ts @@ -2,8 +2,8 @@ import { join } from 'path'; import { BuilderContext, BuilderRun, ScheduleOptions, Target } from '@angular-devkit/architect'; import { JsonObject, logging } from '@angular-devkit/core'; -import { BuildTarget, FSHost, FirebaseDeployConfig, FirebaseTools } from '../interfaces'; -import deploy, { assertSafeDependencyName, assertSupportedPackageManager, deployToFunction, findPackageVersion, processHost } from './actions' +import { BuildTarget, DeployBuilderSchema, FSHost, FirebaseDeployConfig, FirebaseTools } from '../interfaces'; +import deploy, { assertSafeDependencyName, assertSupportedPackageManager, buildCloudRunBuildsSubmitArgs, buildCloudRunDeployArgs, deployToFunction, findPackageVersion, processHost } from './actions' import 'jasmine'; let context: BuilderContext; @@ -301,6 +301,47 @@ describe('universal deployment', () => { });*/ }); +describe('Cloud Run gcloud argv construction', () => { + // Regression coverage for the argv-injection fix: these options used to be interpolated + // into a single command string and split on whitespace, so a value containing a space + // would land as extra, unintended argv entries. They're now passed straight through as + // individual array elements. + const INJECTED_REGION = 'us-central1 --set-env-vars=INJECTED=owned'; + const INJECTED_PROJECT = `${FIREBASE_PROJECT} --format=json`; + + it('keeps a region value containing a space as a single --region argument', () => { + const options: DeployBuilderSchema = { firebaseProject: FIREBASE_PROJECT, region: INJECTED_REGION }; + const args = buildCloudRunDeployArgs('my-service', options, []); + + expect(args[args.indexOf('--region') + 1]).toBe(INJECTED_REGION); + expect(args).not.toContain('--set-env-vars=INJECTED=owned'); + }); + + it('keeps a firebaseProject value containing a space as a single --project argument (deploy)', () => { + const options: DeployBuilderSchema = { firebaseProject: INJECTED_PROJECT, region: 'us-central1' }; + const args = buildCloudRunDeployArgs('my-service', options, []); + + expect(args[args.indexOf('--project') + 1]).toBe(INJECTED_PROJECT); + expect(args).not.toContain('--format=json'); + }); + + it('keeps a firebaseProject value containing a space as a single --project argument (builds submit)', () => { + const options: DeployBuilderSchema = { firebaseProject: INJECTED_PROJECT }; + const args = buildCloudRunBuildsSubmitArgs('cloudRunOut', 'my-service', options); + + expect(args[args.indexOf('--project') + 1]).toBe(INJECTED_PROJECT); + expect(args).not.toContain('--format=json'); + }); + + it('passes cloudRunOptions through as their own argv entries', () => { + const options: DeployBuilderSchema = { firebaseProject: FIREBASE_PROJECT, region: 'us-central1' }; + const args = buildCloudRunDeployArgs('my-service', options, ['--vpc-connector', 'my-connector --unset-env-vars=OWNED']); + + expect(args[args.indexOf('--vpc-connector') + 1]).toBe('my-connector --unset-env-vars=OWNED'); + expect(args).not.toContain('--unset-env-vars=OWNED'); + }); +}); + describe('deploy input validation (command-injection hardening)', () => { describe('assertSupportedPackageManager', () => { ['npm', 'yarn', 'pnpm', 'cnpm', 'bun'].forEach((pm) => { diff --git a/src/schematics/deploy/actions.ts b/src/schematics/deploy/actions.ts index 124aef857..df8a322fc 100644 --- a/src/schematics/deploy/actions.ts +++ b/src/schematics/deploy/actions.ts @@ -28,11 +28,11 @@ const DEFAULT_CLOUD_RUN_OPTIONS: Partial = { const spawnAsync = async ( command: string, + args: string[], options?: SpawnOptionsWithoutStdio ) => new Promise((resolve, reject) => { - const [spawnCommand, ...args] = command.split(/\s+/); - const spawnProcess = spawn(spawnCommand, args, options); + const spawnProcess = spawn(command, args, options); const chunks: Buffer[] = []; const errorChunks: Buffer[] = []; spawnProcess.stdout.on('data', (data) => { @@ -47,7 +47,7 @@ const spawnAsync = async ( reject(error); }); spawnProcess.on('close', (code) => { - if (code === 1) { + if (code !== 0) { reject(Buffer.concat(errorChunks).toString()); return; } @@ -349,6 +349,34 @@ export const deployToFunction = async ( }; +// Exported (rather than kept private) so the argv shape can be asserted directly in tests, +// without having to mock child_process.spawn. +export const buildCloudRunBuildsSubmitArgs = ( + cloudRunOut: string, + serviceId: string, + options: DeployBuilderOptions +): string[] => [ + 'builds', 'submit', cloudRunOut, + '--tag', `gcr.io/${options.firebaseProject}/${serviceId}`, + '--project', options.firebaseProject, + '--quiet', +]; + +export const buildCloudRunDeployArgs = ( + serviceId: string, + options: DeployBuilderOptions, + deployArguments: string[] +): string[] => [ + 'run', 'deploy', serviceId, + '--image', `gcr.io/${options.firebaseProject}/${serviceId}`, + '--project', options.firebaseProject, + ...deployArguments, + '--platform', 'managed', + '--allow-unauthenticated', + '--region', options.region, + '--quiet', +]; + export const deployToCloudRun = async ( firebaseTools: FirebaseTools, context: BuilderContext, @@ -423,25 +451,23 @@ export const deployToCloudRun = async ( throw new SchematicsException('Cloud Run preview not supported.'); } - const deployArguments: any[] = []; + const deployArguments: string[] = []; const cloudRunOptions = options.cloudRunOptions || {}; Object.entries(DEFAULT_CLOUD_RUN_OPTIONS).forEach(([k, v]) => { cloudRunOptions[k] ||= v; }); // lean on the schema for validation (rather than sanitize) - if (cloudRunOptions.cpus) { deployArguments.push('--cpu', cloudRunOptions.cpus); } - if (cloudRunOptions.maxConcurrency) { deployArguments.push('--concurrency', cloudRunOptions.maxConcurrency); } - if (cloudRunOptions.maxInstances) { deployArguments.push('--max-instances', cloudRunOptions.maxInstances); } - if (cloudRunOptions.memory) { deployArguments.push('--memory', cloudRunOptions.memory); } - if (cloudRunOptions.minInstances) { deployArguments.push('--min-instances', cloudRunOptions.minInstances); } - if (cloudRunOptions.timeout) { deployArguments.push('--timeout', cloudRunOptions.timeout); } + if (cloudRunOptions.cpus) { deployArguments.push('--cpu', cloudRunOptions.cpus.toString()); } + if (cloudRunOptions.maxConcurrency) { deployArguments.push('--concurrency', cloudRunOptions.maxConcurrency.toString()); } + if (cloudRunOptions.maxInstances) { deployArguments.push('--max-instances', cloudRunOptions.maxInstances.toString()); } + if (cloudRunOptions.memory) { deployArguments.push('--memory', cloudRunOptions.memory.toString()); } + if (cloudRunOptions.minInstances) { deployArguments.push('--min-instances', cloudRunOptions.minInstances.toString()); } + if (cloudRunOptions.timeout) { deployArguments.push('--timeout', cloudRunOptions.timeout.toString()); } if (cloudRunOptions.vpcConnector) { deployArguments.push('--vpc-connector', cloudRunOptions.vpcConnector); } - // TODO validate serviceId, firebaseProject, and vpcConnector both to limit errors and opp for injection - context.logger.info(`📦 Deploying to Cloud Run`); - await spawnAsync(`gcloud builds submit ${cloudRunOut} --tag gcr.io/${options.firebaseProject}/${serviceId} --project ${options.firebaseProject} --quiet`); - await spawnAsync(`gcloud run deploy ${serviceId} --image gcr.io/${options.firebaseProject}/${serviceId} --project ${options.firebaseProject} ${deployArguments.join(' ')} --platform managed --allow-unauthenticated --region=${options.region} --quiet`); + await spawnAsync('gcloud', buildCloudRunBuildsSubmitArgs(cloudRunOut, serviceId, options)); + await spawnAsync('gcloud', buildCloudRunDeployArgs(serviceId, options, deployArguments)); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const siteTarget = options.target ?? context.target!.project; @@ -475,7 +501,7 @@ export default async function deploy( } if (!firebaseToken && process.env.GOOGLE_APPLICATION_CREDENTIALS) { - await spawnAsync(`gcloud auth activate-service-account --key-file ${process.env.GOOGLE_APPLICATION_CREDENTIALS}`); + await spawnAsync('gcloud', ['auth', 'activate-service-account', '--key-file', process.env.GOOGLE_APPLICATION_CREDENTIALS]); console.log(`Using Google Application Credentials.`); } diff --git a/src/schematics/deploy/schema.json b/src/schematics/deploy/schema.json index 6335d3a8d..ae0b88968 100644 --- a/src/schematics/deploy/schema.json +++ b/src/schematics/deploy/schema.json @@ -51,7 +51,8 @@ }, "functionName": { "type": "string", - "description": "The name of the Cloud Function or Cloud Run serviceId to deploy SSR to" + "pattern": "^[A-Za-z][A-Za-z0-9_-]{0,62}$", + "description": "The name of the Cloud Function or Cloud Run serviceId to deploy SSR to. Must start with a letter and contain only letters, numbers, hyphens and underscores; on the Cloud Functions path this value also becomes a JavaScript identifier, so use only letters and numbers there." }, "functionsNodeVersion": { "oneOf": [{ "type": "number" }, { "type": "string" }], @@ -63,7 +64,8 @@ }, "region": { "type": "string", - "description": "The region to deploy Cloud Functions or Cloud Run to" + "pattern": "^[a-z]+-[a-z]+\\d+$", + "description": "The region to deploy Cloud Functions or Cloud Run to, e.g. us-central1" }, "outputPath": { "type": "string", From e9b74ba37bbdcd12598722d80267a9995a5bac0c Mon Sep 17 00:00:00 2001 From: Francesco Colamonici <4931297+fr-esco@users.noreply.github.com> Date: Tue, 18 Aug 2026 08:35:13 +0200 Subject: [PATCH 03/11] fix(schematics): use a POSIX path for the Cloud Run main.js entry (#3274) Running ng deploy from Windows with the Cloud Run SSR option wrote the generated run/package.json entry paths with backslashes, which the Linux-based Cloud Run runtime cannot resolve. The manifest path is now built with forward slashes explicitly, since the target runtime is Linux regardless of the deploying machine. Fixes #3098 Co-authored-by: Armando Navarro (cherry picked from commit 73acea731bde51e06408d4551edaec19bf9ae81e) --- src/schematics/deploy/actions.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/schematics/deploy/actions.ts b/src/schematics/deploy/actions.ts index df8a322fc..1412539ab 100644 --- a/src/schematics/deploy/actions.ts +++ b/src/schematics/deploy/actions.ts @@ -419,7 +419,8 @@ export const deployToCloudRun = async ( fsHost.copySync(staticOut, newStaticOut); fsHost.copySync(serverOut, newServerOut); - const packageJson = getPackageJson(context, workspaceRoot, options, join(serverBuildOptions.outputPath, 'main.js')); + // Target runtime is Linux based. + const packageJson = getPackageJson(context, workspaceRoot, options, [serverBuildOptions.outputPath, 'main.js'].join('/')); const nodeVersion = packageJson.engines.node; if (!satisfies(process.versions.node, nodeVersion.toString())) { From 9798896d84a985a07ba0f5029757b2603e691e98 Mon Sep 17 00:00:00 2001 From: herdiyanitdev <82978131+herdiyana256@users.noreply.github.com> Date: Fri, 21 Aug 2026 01:31:27 +0700 Subject: [PATCH 04/11] fix(deploy): prevent code-generation injection from angular.json values (#3739) The SSR deploy builders interpolate several angular.json values into generated artifacts that are later executed: a server build target's outputPath into the Cloud Function index.js and the Cloud Run package.json start script, functionName into the exports assignment, region into the .region() call, and functionsNodeVersion into the Cloud Run Dockerfile FROM line. region is escaped structurally with JSON.stringify in the template, and the start script now quotes its path, so a shell no longer splits or expands it. On top of that, outputPath, functionName and functionsNodeVersion are screened before code generation (assertSafeOutputPath, assertSafeFunctionName, assertSafeNodeVersion). assertSafeOutputPath rejects only what is still live once the start script is quoted: quotes, a backslash and line terminators, which break out of the require() string literal, `$` and a backtick, which are still command substitution inside double quotes, and a leading dash, which node reads as a flag. functionName is only screened on the Functions path, where it becomes a JavaScript identifier. functionsNodeVersion is screened against Docker's tag grammar, since the value is a node image tag, with latest excluded because its slim variant is published as node:slim. The functionName and region schema patterns are not repeated here. Stricter versions of both already landed on main in #3726. The functionsNodeVersion schema pattern stays here and matches the runtime check exactly. The TODO above the gcloud calls is restored in narrowed form, covering the values that are still unvalidated: firebaseProject, vpcConnector, and the outputPath deploy option. (cherry picked from commit fb6796b5779333976faeb940a2f1ef40ba685799) --- src/schematics/deploy/actions.jasmine.ts | 205 +++++++++++++++++- src/schematics/deploy/actions.ts | 64 ++++++ .../deploy/functions-templates.jasmine.ts | 24 ++ src/schematics/deploy/functions-templates.ts | 7 +- src/schematics/deploy/schema.json | 3 +- 5 files changed, 299 insertions(+), 4 deletions(-) diff --git a/src/schematics/deploy/actions.jasmine.ts b/src/schematics/deploy/actions.jasmine.ts index b34603b71..6e6e1db86 100644 --- a/src/schematics/deploy/actions.jasmine.ts +++ b/src/schematics/deploy/actions.jasmine.ts @@ -1,9 +1,10 @@ /* eslint-disable @typescript-eslint/no-empty-function */ import { join } from 'path'; +import { Script } from 'vm'; import { BuilderContext, BuilderRun, ScheduleOptions, Target } from '@angular-devkit/architect'; import { JsonObject, logging } from '@angular-devkit/core'; import { BuildTarget, DeployBuilderSchema, FSHost, FirebaseDeployConfig, FirebaseTools } from '../interfaces'; -import deploy, { assertSafeDependencyName, assertSupportedPackageManager, buildCloudRunBuildsSubmitArgs, buildCloudRunDeployArgs, deployToFunction, findPackageVersion, processHost } from './actions' +import deploy, { assertSafeDependencyName, assertSafeFunctionName, assertSafeNodeVersion, assertSafeOutputPath, assertSupportedPackageManager, buildCloudRunBuildsSubmitArgs, buildCloudRunDeployArgs, deployToCloudRun, deployToFunction, findPackageVersion, processHost } from './actions' import 'jasmine'; let context: BuilderContext; @@ -429,3 +430,205 @@ describe('deploy input validation (command-injection hardening)', () => { }); }); }); + +describe('generated artifact validation (codegen-injection hardening)', () => { + describe('assertSafeOutputPath', () => { + [ + 'dist/browser', 'dist/server', 'dist/my-app/browser', 'out', 'a.b-c_d/e', '../dist/browser', + // Only a shell would act on these, and the one place the path reaches a shell is the + // generated start script, which quotes it. So they are unusual directory names rather + // than a way through, and rejecting them would break a deploy that works today. + 'dist/my app', 'dist/*', 'dist/?pp', 'dist/[ab]', '~/x', 'dist\tserver', 'a;b', 'a|b', + ].forEach((outputPath) => { + it(`allows the valid outputPath ${JSON.stringify(outputPath)}`, () => { + expect(() => assertSafeOutputPath(outputPath, 'proj:server')).not.toThrow(); + }); + }); + + [ + `x'); require('child_process').execSync('id'); ('`, + 'a`id`', 'a$(id)', 'a$HOME', 'a\nb', 'a\rb', 'a"b', 'a\\b', '-rf', + ].forEach((outputPath) => { + it(`rejects the unsafe outputPath ${JSON.stringify(outputPath)}`, () => { + expect(() => assertSafeOutputPath(outputPath, 'proj:server')).toThrowError(/Unsafe outputPath/); + }); + }); + }); + + describe('assertSafeNodeVersion', () => { + // A Docker tag, since the Cloud Run path renders it as `FROM node:-slim`. + [undefined, 18, 20, '18', '18.19', '20.11.1', 'lts', 'current', 'iron', '22-bookworm'].forEach((version) => { + it(`allows the valid functionsNodeVersion ${JSON.stringify(version)}`, () => { + expect(() => assertSafeNodeVersion(version)).not.toThrow(); + }); + }); + + [ + '18-slim\nRUN curl evil | sh', '18 && id', '18;id', '$(id)', '`id`', '18/../x', 'x:y', 'x@sha256', + // Grammatical, but node:latest-slim has never been published. + 'latest', + // Docker caps a tag at 128 characters. + '2'.repeat(129), + ].forEach((version) => { + it(`rejects the unsafe functionsNodeVersion ${JSON.stringify(version)}`, () => { + expect(() => assertSafeNodeVersion(version)).toThrowError(/Unsafe functionsNodeVersion/); + }); + }); + }); + + describe('assertSafeFunctionName', () => { + // These are the names that can actually arrive: the schema pattern for functionName is + // the wider Cloud Run service-ID rule, and this is the JavaScript-identifier rule the + // Cloud Functions path needs on top of it. + [undefined, 'ssr', 'ssrHandler', 'a1', 'my_fn'].forEach((functionName) => { + it(`allows the valid functionName ${JSON.stringify(functionName)}`, () => { + expect(() => assertSafeFunctionName(functionName)).not.toThrow(); + }); + }); + + [`ssr; require('child_process').execSync('id'); var _x`, 'my-fn', 'a b', '1fn', 'a.b', `a'`].forEach((functionName) => { + it(`rejects the unsafe functionName ${JSON.stringify(functionName)}`, () => { + expect(() => assertSafeFunctionName(functionName)).toThrowError(/Unsafe functionName/); + }); + }); + }); +}); + +// Runs a generated index.js against stubs, recording what it required and what it ran, so a +// payload that escaped its context is caught by having executed rather than by how it reads. +const runGeneratedFunction = (source: string) => { + const required: string[] = []; + const executed: string[] = []; + const stub: Record = { + app: () => ({}), + https: { onRequest: (app: unknown) => app }, + execSync: (command: string) => { executed.push(command); return ''; }, + }; + stub.region = () => stub; + stub.runWith = () => stub; + const exports: Record = {}; + const run = () => new Script(source).runInNewContext({ + exports, + module: { exports }, + require: (id: string) => { required.push(id); return stub; }, + }); + return { required, executed, exports, run }; +}; + +// These drive the builders end-to-end so the protection cannot be silently dropped: every +// assertSafe* call site in deployToFunction / deployToCloudRun is covered by a spec here +// that fails if that call is removed, and so is the region escaping in the template. That +// includes the static build target, whose outputPath only ever reaches the filesystem and +// so has nothing exploitable to assert beyond the rejection itself. +describe('generated artifact validation is wired into the builders', () => { + beforeEach(() => initMocks()); + + const withOutputPaths = ( + staticOutputPath: string, + serverOutputPath: string, + ): BuilderContext['getTargetOptions'] => (target: Target) => { + if (target.target === 'build') { return Promise.resolve({ outputPath: staticOutputPath }); } + if (target.target === 'server') { return Promise.resolve({ outputPath: serverOutputPath }); } + // Matches architect, which throws rather than handing back options-less targets. + throw new Error(`Invalid target: ${JSON.stringify(target)}.`); + }; + + const withServerOutputPath = (outputPath: string) => withOutputPaths('dist/browser', outputPath); + const withStaticOutputPath = (outputPath: string) => withOutputPaths(outputPath, 'dist/server'); + + const EVIL_PATH = `dist'); require('child_process').execSync('id'); ('`; + + it('deployToFunction rejects a hostile server outputPath', async () => { + context.getTargetOptions = withServerOutputPath(EVIL_PATH); + await expectAsync(deployToFunction( + firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, + { preview: false }, undefined, fsHost + )).toBeRejectedWithError(/Unsafe outputPath/); + }); + + it('deployToFunction rejects a hostile static outputPath', async () => { + context.getTargetOptions = withStaticOutputPath(EVIL_PATH); + await expectAsync(deployToFunction( + firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, + { preview: false }, undefined, fsHost + )).toBeRejectedWithError(/Unsafe outputPath/); + }); + + it('deployToFunction rejects a server outputPath that starts with a dash', async () => { + context.getTargetOptions = withServerOutputPath('-rf'); + await expectAsync(deployToFunction( + firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, + { preview: false }, undefined, fsHost + )).toBeRejectedWithError(/Unsafe outputPath/); + }); + + it('deployToFunction rejects a functionName that is not a plain identifier', async () => { + await expectAsync(deployToFunction( + firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, + { preview: false, functionName: `ssr; require('child_process').execSync('id'); var _x` }, + undefined, fsHost + )).toBeRejectedWithError(/Unsafe functionName/); + }); + + it('deployToFunction escapes region into the generated function instead of interpolating it raw', async () => { + const spy = spyOn(fsHost, 'writeFileSync'); + const region = `us-central1'); require('child_process').execSync('id'); ('`; + await deployToFunction( + firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, + { preview: false, region }, undefined, fsHost + ); + // By path rather than by call order, so adding or reordering a write does not silently + // point this at the wrong file. + const write = spy.calls.allArgs().find(([path]) => path.endsWith('index.js')); + if (!write) { throw new Error('deployToFunction wrote no index.js'); } + const indexJs = write[1]; + expect(indexJs).toContain(`.region(${JSON.stringify(region)})`); + + // Interpolated raw, the payload closes `.region('` and the require becomes a statement + // of its own, which runs when the function loads. Rendered through the fixed template it + // stays inside a string literal, so running the source touches neither. + const generated = runGeneratedFunction(indexJs); + expect(generated.run).not.toThrow(); + expect(generated.executed).toEqual([]); + expect(generated.required).not.toContain('child_process'); + expect(Object.keys(generated.exports)).toEqual(['ssr']); + }); + + it('deployToCloudRun rejects a hostile server outputPath', async () => { + context.getTargetOptions = withServerOutputPath(EVIL_PATH); + await expectAsync(deployToCloudRun( + firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, + { preview: false }, undefined, fsHost + )).toBeRejectedWithError(/Unsafe outputPath/); + }); + + it('deployToCloudRun rejects a hostile static outputPath', async () => { + context.getTargetOptions = withStaticOutputPath(EVIL_PATH); + await expectAsync(deployToCloudRun( + firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, + { preview: false }, undefined, fsHost + )).toBeRejectedWithError(/Unsafe outputPath/); + }); + + it('deployToCloudRun rejects a hostile functionsNodeVersion', async () => { + await expectAsync(deployToCloudRun( + firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, + { preview: false, functionsNodeVersion: '18-slim\nRUN curl evil | sh' }, undefined, fsHost + )).toBeRejectedWithError(/Unsafe functionsNodeVersion/); + }); + + it('deployToCloudRun rejects a hostile functionsNodeVersion before touching the output directory', async () => { + const removeSpy = spyOn(fsHost, 'removeSync'); + const copySpy = spyOn(fsHost, 'copySync'); + const writeSpy = spyOn(fsHost, 'writeFileSync'); + + await expectAsync(deployToCloudRun( + firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, + { preview: false, functionsNodeVersion: '18-slim\nRUN curl evil | sh' }, undefined, fsHost + )).toBeRejectedWithError(/Unsafe functionsNodeVersion/); + + expect(removeSpy).not.toHaveBeenCalled(); + expect(copySpy).not.toHaveBeenCalled(); + expect(writeSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/src/schematics/deploy/actions.ts b/src/schematics/deploy/actions.ts index 1412539ab..816fb7d58 100644 --- a/src/schematics/deploy/actions.ts +++ b/src/schematics/deploy/actions.ts @@ -59,6 +59,58 @@ export type DeployBuilderOptions = DeployBuilderSchema & Record; const escapeRegExp = (str: string) => str.replace(/[-[\]/{}()*+?.\\^$|]/g, '\\$&'); +// A build target's outputPath (from angular.json's architect...options) +// is interpolated raw into generated Cloud Function source (`require('.//main')`) +// and into the generated package.json start script, which the Cloud Run image runs through +// a shell. That start script quotes the path (functions-templates.ts), so word splitting, +// globbing and `~` expansion are already off; what remains live is the set below. +// +// ' and \ break out of the require() string literal, as do the line terminators, since a +// JavaScript string literal cannot span a line +// " ` and $ stay live inside the double quotes of the start script: `"` closes them, and +// `` ` `` and `$(` are still command substitution in there +// +// A leading dash is rejected separately: quoting does not stop `node "-rf/main.js"` from +// being read as a flag rather than a path. +export const assertSafeOutputPath = (outputPath: string, targetName: string): void => { + if (/['"`\\$\n\r]/.test(outputPath) || outputPath.startsWith('-')) { + throw new SchematicsException( + `Unsafe outputPath ${JSON.stringify(outputPath)} for target '${targetName}' in angular.json.` + ); + } +}; + +// functionName is interpolated raw into the generated Cloud Function source as the +// `exports.` assignment target (functions-templates.ts), which is executed when the +// function loads. Allow only a plain JavaScript identifier so it cannot introduce further +// statements; this also turns a name that would silently produce an unparseable file (for +// example one containing a dash) into an explicit error. +export const assertSafeFunctionName = (functionName: string | undefined): void => { + if (functionName !== undefined && !/^[A-Za-z_$][A-Za-z0-9_$]*$/.test(functionName)) { + throw new SchematicsException( + `Unsafe functionName ${JSON.stringify(functionName)} in angular.json; expected a plain identifier.` + ); + } +}; + +// functionsNodeVersion is interpolated raw into the generated Dockerfile's FROM line +// (`FROM node:-slim`), executed during the Cloud Run container build, so the value +// is a Docker image tag. This is Docker's own tag grammar, which bounds the length at 128 +// and admits none of the characters that would open a new instruction (a line terminator) +// or point FROM at different image content (a space, a slash, a colon or an `@`). `latest` +// is grammatical but has never resolved here: the official image publishes its slim variant +// as node:slim, so node:latest-slim does not exist. +// Kept in step with the functionsNodeVersion pattern in schema.json. +const NODE_IMAGE_TAG = /^(?!latest$)[\w][\w.-]{0,127}$/; + +export const assertSafeNodeVersion = (version: string | number | undefined): void => { + if (version !== undefined && !NODE_IMAGE_TAG.test(String(version))) { + throw new SchematicsException( + `Unsafe functionsNodeVersion ${JSON.stringify(version)} in angular.json; expected a node image tag, such as 22 or lts.` + ); + } +}; + const moveSync = (src: string, dest: string) => { copySync(src, dest); removeSync(src); @@ -242,6 +294,7 @@ export const deployToFunction = async ( `Cannot read the output path option of the Angular project '${staticBuildTarget.name}' in angular.json` ); } + assertSafeOutputPath(staticBuildOptions.outputPath, staticBuildTarget.name); const serverBuildOptions = await context.getTargetOptions(targetFromTargetString(serverBuildTarget.name)); if (!serverBuildOptions.outputPath || typeof serverBuildOptions.outputPath !== 'string') { @@ -249,11 +302,13 @@ export const deployToFunction = async ( `Cannot read the output path option of the Angular project '${serverBuildTarget.name}' in angular.json` ); } + assertSafeOutputPath(serverBuildOptions.outputPath, serverBuildTarget.name); const staticOut = join(workspaceRoot, staticBuildOptions.outputPath); const serverOut = join(workspaceRoot, serverBuildOptions.outputPath); const functionsOut = options.outputPath ? join(workspaceRoot, options.outputPath) : dirname(serverOut); + assertSafeFunctionName(options.functionName); const functionName = options.functionName || DEFAULT_FUNCTION_NAME; const newStaticOut = join(functionsOut, staticBuildOptions.outputPath); @@ -394,6 +449,7 @@ export const deployToCloudRun = async ( `Cannot read the output path option of the Angular project '${staticBuildTarget.name}' in angular.json` ); } + assertSafeOutputPath(staticBuildOptions.outputPath, staticBuildTarget.name); const serverBuildOptions = await context.getTargetOptions(targetFromTargetString(serverBuildTarget.name)); if (!serverBuildOptions.outputPath || typeof serverBuildOptions.outputPath !== 'string') { @@ -401,6 +457,11 @@ export const deployToCloudRun = async ( `Cannot read the output path option of the Angular project '${serverBuildTarget.name}' in angular.json` ); } + assertSafeOutputPath(serverBuildOptions.outputPath, serverBuildTarget.name); + // Checked here, alongside the outputPath screens, rather than next to the Dockerfile it + // guards: everything below wipes and refills the output directory, so rejecting late + // would leave that directory half-written before throwing. + assertSafeNodeVersion(options.functionsNodeVersion); const staticOut = join(workspaceRoot, staticBuildOptions.outputPath); const serverOut = join(workspaceRoot, serverBuildOptions.outputPath); @@ -466,6 +527,9 @@ export const deployToCloudRun = async ( if (cloudRunOptions.timeout) { deployArguments.push('--timeout', cloudRunOptions.timeout.toString()); } if (cloudRunOptions.vpcConnector) { deployArguments.push('--vpc-connector', cloudRunOptions.vpcConnector); } + // TODO validate firebaseProject, vpcConnector, and the outputPath deploy option both to + // limit errors and opp for injection + context.logger.info(`📦 Deploying to Cloud Run`); await spawnAsync('gcloud', buildCloudRunBuildsSubmitArgs(cloudRunOut, serviceId, options)); await spawnAsync('gcloud', buildCloudRunDeployArgs(serviceId, options, deployArguments)); diff --git a/src/schematics/deploy/functions-templates.jasmine.ts b/src/schematics/deploy/functions-templates.jasmine.ts index b6bf4c148..0477ae3c3 100644 --- a/src/schematics/deploy/functions-templates.jasmine.ts +++ b/src/schematics/deploy/functions-templates.jasmine.ts @@ -8,6 +8,17 @@ describe('functions templates', () => { expect(generated).toContain(`require('firebase-functions/v1')`); expect(generated).not.toContain(`require('firebase-functions')`); }); + + it('escapes region rather than interpolating it into a string literal', () => { + const region = `us-central1'); require('child_process').execSync('id'); ('`; + const generated = defaultFunction('dist/app', { region }, undefined); + expect(generated).toContain(`.region(${JSON.stringify(region)})`); + // Interpolated raw, the payload closes `.region('` and the require becomes a statement + // of its own. Escaped, it changes nothing but that one argument, so swapping it back + // out has to reproduce the benign render exactly. + expect(generated.replace(JSON.stringify(region), JSON.stringify('us-central1'))) + .toBe(defaultFunction('dist/app', { region: 'us-central1' }, undefined)); + }); }); describe('defaultPackage', () => { @@ -16,5 +27,18 @@ describe('functions templates', () => { expect(generated.engines.node).toBe(DEFAULT_NODE_VERSION.toString()); expect(DEFAULT_NODE_VERSION).toBe(22); }); + + it('quotes the start script path, which a shell would otherwise split or expand', () => { + // `main` carries a build target's outputPath, and the Cloud Run image runs this + // through `npm start`. + expect(defaultPackage({}, {}, {}, 'dist/my app/main.js').scripts.start) + .toBe('node "dist/my app/main.js"'); + expect(defaultPackage({}, {}, {}, 'dist/[ab]/main.js').scripts.start) + .toBe('node "dist/[ab]/main.js"'); + }); + + it('falls back to the functions shell when there is no main', () => { + expect(defaultPackage({}, {}, {}).scripts.start).toBe('firebase functions:shell'); + }); }); }); diff --git a/src/schematics/deploy/functions-templates.ts b/src/schematics/deploy/functions-templates.ts index b7af2a3b1..f3567545a 100644 --- a/src/schematics/deploy/functions-templates.ts +++ b/src/schematics/deploy/functions-templates.ts @@ -23,7 +23,10 @@ export const defaultPackage = ( description: 'Angular Universal Application', main: main ?? 'index.js', scripts: { - start: main ? `node ${main}` : 'firebase functions:shell', + // Quoted: `npm start` hands this to a shell, and `main` carries a build target's + // outputPath, so an unquoted path with a space in it splits and one with a glob + // character in it expands before node ever sees it. + start: main ? `node "${main}"` : 'firebase functions:shell', }, engines: { node: (options.functionsNodeVersion || DEFAULT_NODE_VERSION).toString() @@ -47,7 +50,7 @@ require("firebase-functions/logger/compat"); const expressApp = require('./${path}/main').app(); exports.${functionName || DEFAULT_FUNCTION_NAME} = functions - .region('${options.region || DEFAULT_FUNCTION_REGION}') + .region(${JSON.stringify(options.region || DEFAULT_FUNCTION_REGION)}) .runWith(${JSON.stringify(options.functionsRuntimeOptions || DEFAULT_RUNTIME_OPTIONS)}) .https .onRequest(expressApp); diff --git a/src/schematics/deploy/schema.json b/src/schematics/deploy/schema.json index ae0b88968..ff8d0bc11 100644 --- a/src/schematics/deploy/schema.json +++ b/src/schematics/deploy/schema.json @@ -56,7 +56,8 @@ }, "functionsNodeVersion": { "oneOf": [{ "type": "number" }, { "type": "string" }], - "description": "Version of Node.js to run Cloud Functions / Run on" + "pattern": "^(?!latest$)[\\w][\\w.-]{0,127}$", + "description": "Version of Node.js to run Cloud Functions / Run on, e.g. 22. On Cloud Run this is the tag of the node base image (node:-slim), so any tag that image publishes works, including lts and 22-bookworm; latest is excluded because its slim variant is published as node:slim. On Cloud Functions the value becomes the engines.node field, which firebase-tools resolves to a nodejs runtime by concatenation, so a plain major version is the only shape that works there. Not a semver range: >=18 and ^20 are rejected outright, and 20.x, though a grammatical tag, matches no published image." }, "CF3v2": { "type": "boolean", From 695045cddc34964c3dfb67cbebca73090ba33dfa Mon Sep 17 00:00:00 2001 From: Armando Navarro Date: Sun, 27 Sep 2026 16:23:11 -0700 Subject: [PATCH 05/11] fix(schematics): read the signed-in account on firebase-tools 15.26+ (#3769) firebase-tools 15.26 runs non-interactively when it detects an AI agent or when stdin is not a terminal, and in that mode login() returns undefined instead of the signed-in account. `ng add` and `ng deploy` crashed reading `.email` from it. getActiveAccount passes `interactive: true`, which returns the account without a prompt once one is signed in. The `firebase login` and `firebase login:add` commands setup starts get `--interactive` for the same reason. Setup no longer passes its options to login(), since they carry the Angular project name, which firebase-tools rejects as a project id for names like `myApp`. The Quickstart stops telling readers to install firebase-tools 14, and says what a stopped run leaves behind. Fixes #3768 The Quickstart changes are left out, since docs are not backported. (cherry picked from commit 38dfc3f42d1e299543aefcebe00dc9eb8811e62d) --- src/schematics/deploy/actions.jasmine.ts | 33 +++++++++++++++++++++++- src/schematics/deploy/actions.ts | 20 +++++++++++--- src/schematics/firebaseTools.ts | 8 ++++++ src/schematics/interfaces.ts | 2 +- src/schematics/setup/index.ts | 4 +-- src/schematics/setup/prompts.ts | 17 +++++++----- 6 files changed, 71 insertions(+), 13 deletions(-) diff --git a/src/schematics/deploy/actions.jasmine.ts b/src/schematics/deploy/actions.jasmine.ts index 6e6e1db86..496e4c96f 100644 --- a/src/schematics/deploy/actions.jasmine.ts +++ b/src/schematics/deploy/actions.jasmine.ts @@ -115,8 +115,14 @@ const initMocks = () => { describe('Deploy Angular apps', () => { beforeEach(() => initMocks()); + /** Spies on `login()` and keeps a `login.list()`, which `spyOn` would otherwise drop. */ + const spyOnLogin = (accounts: { user: Record }[]) => + Object.assign(spyOn(firebaseMock, 'login'), { list: () => Promise.resolve(accounts), add: login.add, use: login.use }); + + const signedIn = [{ user: { email: 'foo@bar.baz' } }]; + it('should call login', async () => { - const spy = spyOn(firebaseMock, 'login').and.resolveTo({ email: 'foo@bar.baz' }); + const spy = spyOnLogin(signedIn).and.resolveTo({ email: 'foo@bar.baz' }); await deploy( firebaseMock, context, STATIC_BUILD_TARGET, undefined, undefined, undefined, { projectId: FIREBASE_PROJECT, preview: false } @@ -124,6 +130,31 @@ describe('Deploy Angular apps', () => { expect(spy).toHaveBeenCalled(); }); + it('should read the signed-in account with interactive, which firebase-tools 15.26+ needs under an AI agent', async () => { + const spy = spyOnLogin(signedIn).and.resolveTo({ email: 'foo@bar.baz' }); + await deploy( + firebaseMock, context, STATIC_BUILD_TARGET, undefined, + undefined, undefined, { projectId: FIREBASE_PROJECT, preview: false } + ); + expect(spy).toHaveBeenCalledWith(jasmine.objectContaining({ interactive: true })); + }); + + it('should deploy when login returns no account for a signed-in user', async () => { + spyOnLogin(signedIn).and.resolveTo(undefined); + await expectAsync(deploy( + firebaseMock, context, STATIC_BUILD_TARGET, undefined, + undefined, undefined, { projectId: FIREBASE_PROJECT, preview: false } + )).toBeResolved(); + }); + + it('should say to run firebase login when no account is signed in', async () => { + spyOnLogin([]).and.resolveTo(undefined); + await expectAsync(deploy( + firebaseMock, context, STATIC_BUILD_TARGET, undefined, + undefined, undefined, { projectId: FIREBASE_PROJECT, preview: false } + )).toBeRejectedWithError(/Run `firebase login`/); + }); + it('should not call login', async () => { const spy = spyOn(firebaseMock, 'login'); await deploy(firebaseMock, context, STATIC_BUILD_TARGET, undefined, undefined, undefined, { preview: false }, FIREBASE_TOKEN); diff --git a/src/schematics/deploy/actions.ts b/src/schematics/deploy/actions.ts index 816fb7d58..49a7e5db5 100644 --- a/src/schematics/deploy/actions.ts +++ b/src/schematics/deploy/actions.ts @@ -10,6 +10,7 @@ import open from 'open'; import { satisfies } from 'semver'; import tripleBeam from 'triple-beam'; import * as winston from 'winston'; +import { getActiveAccount } from '../firebaseTools.js'; import { BuildTarget, CloudRunOptions, DeployBuilderSchema, FSHost, FirebaseTools } from '../interfaces'; import { firebaseFunctionsDependencies } from '../versions.json'; import { DEFAULT_FUNCTION_NAME, defaultFunction, defaultPackage, dockerfile, functionGen2 } from './functions-templates'; @@ -547,6 +548,12 @@ export const deployToCloudRun = async ( }); }; +/** Whether the Firebase CLI has at least one signed-in account. */ +const isSignedIn = async (firebaseTools: FirebaseTools) => { + const accounts = await firebaseTools.login.list(); + return Array.isArray(accounts) && accounts.length > 0; +}; + export default async function deploy( firebaseTools: FirebaseTools, context: BuilderContext, @@ -560,9 +567,16 @@ export default async function deploy( const legacyNgDeploy = !options.version || options.version < 2; if (!firebaseToken && !process.env.GOOGLE_APPLICATION_CREDENTIALS) { - await firebaseTools.login(); - const user = await firebaseTools.login({ projectRoot: context.workspaceRoot }); - console.log(`Logged into Firebase as ${user.email}.`); + if (!await isSignedIn(firebaseTools)) { + await firebaseTools.login(); + if (!await isSignedIn(firebaseTools)) { + throw new Error('No Firebase account is signed in. Run `firebase login`, then run `ng deploy` again.'); + } + } + const user = await getActiveAccount(firebaseTools, context.workspaceRoot); + if (user) { + console.log(`Logged into Firebase as ${user.email}.`); + } } if (!firebaseToken && process.env.GOOGLE_APPLICATION_CREDENTIALS) { diff --git a/src/schematics/firebaseTools.ts b/src/schematics/firebaseTools.ts index 3d8136763..cd691f9e2 100644 --- a/src/schematics/firebaseTools.ts +++ b/src/schematics/firebaseTools.ts @@ -8,6 +8,14 @@ declare global { var firebaseTools: FirebaseTools|undefined; } +/** + * The account firebase-tools uses in `projectRoot`. Call it only once an account is signed in. + * Without `interactive`, firebase-tools 15.26+ returns undefined whenever it runs non-interactively, + * as it does under an AI agent. + */ +export const getActiveAccount = (firebaseTools: FirebaseTools, projectRoot: string) => + firebaseTools.login({ projectRoot, interactive: true }); + export const getFirebaseTools = () => globalThis.firebaseTools ? Promise.resolve(globalThis.firebaseTools) : new Promise((resolve, reject) => { diff --git a/src/schematics/interfaces.ts b/src/schematics/interfaces.ts index a3a14f3be..0f2e22b32 100644 --- a/src/schematics/interfaces.ts +++ b/src/schematics/interfaces.ts @@ -125,7 +125,7 @@ export interface FirebaseTools { list(): Promise<{user: Record}[] | { users: undefined }>; add(): Promise>; use(email: string, options?: unknown): Promise; - } & ((options?: unknown) => Promise>); + } & ((options?: unknown) => Promise | undefined>); deploy(config: FirebaseDeployConfig): Promise; diff --git a/src/schematics/setup/index.ts b/src/schematics/setup/index.ts index 8a657e17a..d697318ee 100644 --- a/src/schematics/setup/index.ts +++ b/src/schematics/setup/index.ts @@ -3,7 +3,7 @@ import { join } from 'path'; import { asWindowsPath, normalize } from '@angular-devkit/core'; import { SchematicContext, Tree, chain } from '@angular-devkit/schematics'; import { addRootProvider } from '@schematics/angular/utility'; -import { getFirebaseTools } from '../firebaseTools'; +import { getActiveAccount, getFirebaseTools } from '../firebaseTools'; import { DataConnectConnectorConfig, DeployOptions, FEATURES, FirebaseApp, FirebaseJSON, FirebaseProject, @@ -86,7 +86,7 @@ export const ngAddSetupProject = ( ); const user = await userPrompt({ projectRoot }); - const defaultUser = await firebaseTools.login(options); + const defaultUser = await getActiveAccount(firebaseTools, projectRoot); if (user.email !== defaultUser?.email) { await firebaseTools.login.use(user.email, { projectRoot }); } diff --git a/src/schematics/setup/prompts.ts b/src/schematics/setup/prompts.ts index b48d80d5a..356090664 100644 --- a/src/schematics/setup/prompts.ts +++ b/src/schematics/setup/prompts.ts @@ -1,7 +1,7 @@ import { spawnSync } from 'child_process'; import * as fuzzy from 'fuzzy'; import * as inquirer from 'inquirer'; -import { getFirebaseTools } from '../firebaseTools'; +import { getActiveAccount, getFirebaseTools } from '../firebaseTools'; import { FEATURES, FirebaseApp, FirebaseProject, featureOptions } from '../interfaces'; import { shortAppId } from '../utils'; @@ -96,10 +96,15 @@ export const userPrompt = async (options: { projectRoot: string }): Promise ({ name: user.email, value: user })); const newChoice = { name: '[Login in with another account]', value: NEW_OPTION }; const { user } = await inquirer.prompt({ @@ -107,10 +112,10 @@ export const userPrompt = async (options: { projectRoot: string }): Promise it.value.email === defaultUser.email)?.value, + default: choices.find(it => it.value.email === defaultUser?.email)?.value, }) as any; if (user === NEW_OPTION) { - spawnSync('firebase login:add', { shell: true, cwd: options.projectRoot, stdio: 'inherit' }); + spawnSync('firebase login:add --interactive', { shell: true, cwd: options.projectRoot, stdio: 'inherit' }); loginList = await firebaseTools.login.list(); if (!Array.isArray(loginList)) { throw new Error("firebase login:list did not respond as expected"); From 3c3777377c35e507beeb29859d2905be2810718d Mon Sep 17 00:00:00 2001 From: Armando Navarro Date: Sun, 4 Oct 2026 17:13:17 -0700 Subject: [PATCH 06/11] fix(ci): keep the canary dist-tag from moving back to an older build (#3784) Every canary publish moved the `canary` dist-tag, whatever commit it was built from. Two merges close together could publish out of order, and a re-run of an older run could publish its build last. The publish job now clones the repository's commit history and publishes a canary only when its commit comes after the commit of the canary on npm, or is the same commit under a new version. A build of an earlier commit is skipped with a warning. Commits are compared rather than versions, because a version can be higher for an older commit. Canary publishes also run one at a time, queued, so each check reads what the previous publish left on npm. Release publishes are unchanged. A scheduled run on an unchanged main now skips with a notice instead of failing on the duplicate version. Fixes #3783 (cherry picked from commit eee34866c42241055ade0e4ecb215b5784ddefc8) --- .github/workflows/test.yml | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f52ae128e..666b0d126 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -163,6 +163,11 @@ jobs: name: Publish (NPM) needs: ['build', 'test', 'browser'] if: ${{ github.ref == 'refs/heads/main' || github.event_name == 'release' }} + # One canary publish at a time, so the check below reads the canary the previous one published. + concurrency: + group: ${{ github.event_name == 'release' && github.run_id || 'canary-publish' }} + cancel-in-progress: false + queue: max steps: - name: Setup node uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 @@ -175,7 +180,37 @@ jobs: with: name: angularfire-${{ github.run_id }} path: dist + - name: Skip a canary that is not newer than npm's + id: canary_check + run: | + VERSION=$(node -p "require('./dist/packages-dist/package.json').version") + if [[ $VERSION == *-canary.* ]]; then + # The dist-tags endpoint is not CDN-cached, unlike the package data `npm view` reads. + DIST_TAGS=$(curl -fsS --retry 3 --max-time 30 https://registry.npmjs.org/-/package/@angular/fire/dist-tags) + NPM_CANARY=$(node -p "JSON.parse(process.argv[1]).canary" "$DIST_TAGS") + # Order by position on main, not by version, which can be higher for an older commit. + git clone --quiet --bare --filter=tree:0 "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY" history.git + HOW_TO_FIX="Every canary publish fails until the canary dist-tag points at a build of a commit on main. Publish rights are required to fix it with: npm dist-tag add @angular/fire@ canary" + if ! NPM_CANARY_COMMIT=$(git -C history.git rev-parse --verify --quiet "${NPM_CANARY##*[.-]}^{commit}"); then + echo "::error::Could not match the canary on npm, $NPM_CANARY, to a single commit in this repository. $HOW_TO_FIX" + exit 1 + fi + # `npm publish` always moves a dist-tag, so a canary that is not newer must not publish at all. + if [[ $NPM_CANARY_COMMIT == "$GITHUB_SHA" ]]; then + if [[ $VERSION == "$NPM_CANARY" ]]; then + echo "::notice::Not publishing $VERSION, because it is already the canary on npm." + echo "skip=true" >> "$GITHUB_OUTPUT" + fi + elif git -C history.git merge-base --is-ancestor "$GITHUB_SHA" "$NPM_CANARY_COMMIT"; then + echo "::warning::Not publishing $VERSION, because the canary on npm, $NPM_CANARY, is from a later commit on main." + echo "skip=true" >> "$GITHUB_OUTPUT" + elif ! git -C history.git merge-base --is-ancestor "$NPM_CANARY_COMMIT" "$GITHUB_SHA"; then + echo "::error::Not publishing $VERSION, because its commit and the commit of the canary on npm, $NPM_CANARY, are not on the same line of history. One of them is not on main. $HOW_TO_FIX" + exit 1 + fi + fi - name: Publish + if: steps.canary_check.outputs.skip != 'true' run: | cd ./dist/packages-dist chmod +x publish.sh From ac42ffa65b3da4fec9d968e64d1157921f80cbab Mon Sep 17 00:00:00 2001 From: Armando Navarro Date: Fri, 9 Oct 2026 11:40:33 -0700 Subject: [PATCH 07/11] fix(core): drop the unused optional @angular/platform-server peer (#3792) npm 10, and npm 11 before 11.20, resolve a missing optional peer anyway while installing. For an app whose package.json uses caret ranges and whose Angular is one patch behind the newest, npm picks the newest @angular/platform-server, which requires that exact newest @angular/core, and the install stops with ERESOLVE. Every current Node line bundles one of those npm versions. Nothing in the published package imports @angular/platform-server, so the peer is removed. Apps that render on the server still install it themselves, as they already do. (cherry picked from commit bbce8f09ce6f75f9d3ec511deefd334bb2fa9814) --- src/package.json | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/package.json b/src/package.json index a2068ed19..a9490e97b 100644 --- a/src/package.json +++ b/src/package.json @@ -30,13 +30,11 @@ "@angular/common": "^20.0.0", "@angular/core": "^20.0.0", "@angular/platform-browser": "^20.0.0", - "@angular/platform-server": "^20.0.0", "rxjs": "~7.8.0", "firebase-tools": "^14.0.0 || ^15.0.0" }, "peerDependenciesMeta": { - "firebase-tools": { "optional": true }, - "@angular/platform-server": { "optional": true } + "firebase-tools": { "optional": true } }, "dependencies": { "firebase": "^11.8.0", From 72a74eb6788969a72dddac18144353afebfc603b Mon Sep 17 00:00:00 2001 From: Armando Navarro Date: Fri, 9 Oct 2026 11:31:26 -0700 Subject: [PATCH 08/11] fix(ci): choose a release's dist-tag when it publishes, and move next after it (#3794) * fix(ci): choose a release's dist-tag when it publishes, and move next after it A stable tag published to latest unconditionally, so a 20.x release tagged after 21.0.0 would move latest back to 20.x. A stable release now goes to latest only when it ranks above npm's latest and every other stable git tag, since a higher release can still be in npm's publish-time malware scan. Otherwise it goes to v-lts, and the job fails if it does not rank above that tag or another release of its major. Every publish now waits until npm lists it, in one npm-publish queue, so the next publish reads the real dist-tags. That stops a queued canary from reading a stale canary tag. After a release reaches latest, next moves up to it, and a failure there prints the npm dist-tag command that finishes it. A re-run of a published release skips npm publish and goes on to the wait and the next move. The steps live in tools/publish-job.js and tools/release-tag.js so specs cover them, and build.sh no longer picks a tag. * refactor(ci): move the canary check into tools/publish-job.js The check that skips a canary of an earlier commit keeps the same decisions and messages, and its specs now cover each branch. It runs only for pushes and scheduled runs, and a failed read of npm's dist-tags now says why. (cherry picked from commit 31fb2ae8fc84bf410b5195840ba34766679854a1) --- .github/workflows/test.yml | 67 ++++---- .gitignore | 1 - .npmignore | 1 - tools/build.sh | 11 +- tools/publish-job.jasmine.ts | 297 +++++++++++++++++++++++++++++++++++ tools/publish-job.js | 176 +++++++++++++++++++++ tools/release-tag.jasmine.ts | 131 +++++++++++++++ tools/release-tag.js | 82 ++++++++++ 8 files changed, 720 insertions(+), 46 deletions(-) create mode 100644 tools/publish-job.jasmine.ts create mode 100644 tools/publish-job.js create mode 100644 tools/release-tag.jasmine.ts create mode 100644 tools/release-tag.js diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 666b0d126..4f9715239 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -163,12 +163,22 @@ jobs: name: Publish (NPM) needs: ['build', 'test', 'browser'] if: ${{ github.ref == 'refs/heads/main' || github.event_name == 'release' }} - # One canary publish at a time, so the check below reads the canary the previous one published. + # One npm publish at a time, each held until npm lists it (about 20 minutes at most), so every + # check below reads what the previous publish wrote. + timeout-minutes: 30 concurrency: - group: ${{ github.event_name == 'release' && github.run_id || 'canary-publish' }} + group: npm-publish cancel-in-progress: false queue: max steps: + - name: Checkout the publish scripts + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + sparse-checkout: | + tools/publish-job.js + tools/release-tag.js + sparse-checkout-cone-mode: false - name: Setup node uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: @@ -182,38 +192,27 @@ jobs: path: dist - name: Skip a canary that is not newer than npm's id: canary_check - run: | - VERSION=$(node -p "require('./dist/packages-dist/package.json').version") - if [[ $VERSION == *-canary.* ]]; then - # The dist-tags endpoint is not CDN-cached, unlike the package data `npm view` reads. - DIST_TAGS=$(curl -fsS --retry 3 --max-time 30 https://registry.npmjs.org/-/package/@angular/fire/dist-tags) - NPM_CANARY=$(node -p "JSON.parse(process.argv[1]).canary" "$DIST_TAGS") - # Order by position on main, not by version, which can be higher for an older commit. - git clone --quiet --bare --filter=tree:0 "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY" history.git - HOW_TO_FIX="Every canary publish fails until the canary dist-tag points at a build of a commit on main. Publish rights are required to fix it with: npm dist-tag add @angular/fire@ canary" - if ! NPM_CANARY_COMMIT=$(git -C history.git rev-parse --verify --quiet "${NPM_CANARY##*[.-]}^{commit}"); then - echo "::error::Could not match the canary on npm, $NPM_CANARY, to a single commit in this repository. $HOW_TO_FIX" - exit 1 - fi - # `npm publish` always moves a dist-tag, so a canary that is not newer must not publish at all. - if [[ $NPM_CANARY_COMMIT == "$GITHUB_SHA" ]]; then - if [[ $VERSION == "$NPM_CANARY" ]]; then - echo "::notice::Not publishing $VERSION, because it is already the canary on npm." - echo "skip=true" >> "$GITHUB_OUTPUT" - fi - elif git -C history.git merge-base --is-ancestor "$GITHUB_SHA" "$NPM_CANARY_COMMIT"; then - echo "::warning::Not publishing $VERSION, because the canary on npm, $NPM_CANARY, is from a later commit on main." - echo "skip=true" >> "$GITHUB_OUTPUT" - elif ! git -C history.git merge-base --is-ancestor "$NPM_CANARY_COMMIT" "$GITHUB_SHA"; then - echo "::error::Not publishing $VERSION, because its commit and the commit of the canary on npm, $NPM_CANARY, are not on the same line of history. One of them is not on main. $HOW_TO_FIX" - exit 1 - fi - fi + if: github.event_name != 'release' + run: node tools/publish-job.js canary-check + - name: Choose the release's dist-tag + id: release_tag + if: github.event_name == 'release' + run: node tools/publish-job.js release-tag - name: Publish - if: steps.canary_check.outputs.skip != 'true' - run: | - cd ./dist/packages-dist - chmod +x publish.sh - ./publish.sh + id: publish + if: steps.canary_check.outputs.skip != 'true' && steps.release_tag.outputs.published != 'true' + run: npm publish ./dist/packages-dist --access public --registry https://wombat-dressing-room.appspot.com --tag "$NPM_TAG" + env: + NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + NPM_TAG: ${{ steps.release_tag.outputs.tag || 'canary' }} + - name: Wait for npm to list the version + id: wait + if: steps.publish.outcome == 'success' || steps.release_tag.outputs.published == 'true' + run: node tools/publish-job.js wait "$NPM_TAG" + env: + NPM_TAG: ${{ steps.release_tag.outputs.tag || 'canary' }} + - name: Move next up to the release + if: steps.wait.outputs.listed == 'true' && steps.release_tag.outputs.tag == 'latest' + run: node tools/publish-job.js move-next env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} diff --git a/.gitignore b/.gitignore index beb5d455e..0e6c87259 100644 --- a/.gitignore +++ b/.gitignore @@ -21,7 +21,6 @@ coverage api-*.json angularfire.tgz unpack.sh -publish.sh .firebase .angular .vscode \ No newline at end of file diff --git a/.npmignore b/.npmignore index 02a57a315..f1e38dce4 100644 --- a/.npmignore +++ b/.npmignore @@ -1,6 +1,5 @@ *.spec.* test-config.* -publish.sh __ivy_ngcc__/ *.min.js *.min.js.map diff --git a/tools/build.sh b/tools/build.sh index f92330a27..37db29c6a 100755 --- a/tools/build.sh +++ b/tools/build.sh @@ -1,21 +1,12 @@ SHORT_SHA=$(git rev-parse --short $GITHUB_SHA) TAG_TEST="^refs/tags/.+$" -LATEST_TEST="^[^-]*$" if [[ $GITHUB_REF =~ $TAG_TEST ]]; then OVERRIDE_VERSION=${GITHUB_REF/refs\/tags\//} - if [[ $OVERRIDE_VERSION =~ $LATEST_TEST ]]; then - NPM_TAG=latest - else - NPM_TAG=next - fi; else OVERRIDE_VERSION=$(node -e "console.log(require('./package.json').version)")-canary.$SHORT_SHA - NPM_TAG=canary fi; npm --no-git-tag-version --allow-same-version -f version $OVERRIDE_VERSION -npm run build && - echo "npm publish . --access public --registry https://wombat-dressing-room.appspot.com --tag $NPM_TAG" > ./dist/packages-dist/publish.sh && - chmod +x ./dist/packages-dist/publish.sh +npm run build diff --git a/tools/publish-job.jasmine.ts b/tools/publish-job.jasmine.ts new file mode 100644 index 000000000..0bd7145ab --- /dev/null +++ b/tools/publish-job.jasmine.ts @@ -0,0 +1,297 @@ +import { execFileSync } from 'child_process'; +import { mkdtempSync, readFileSync, writeFileSync } from 'fs'; +import fsExtra from 'fs-extra'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import * as publishJob from './publish-job.js'; +import 'jasmine'; + +type DistTags = Record; + +interface FakeActionsOptions { + version: string; + // One entry per read of npm's dist-tags. The last entry repeats. + distTagReads: (DistTags | Error)[]; + // Leave out to make any read of the repository's tags fail the spec. + gitTags?: string[]; + npmFails?: boolean; + // The commit being published, and main's history as each commit's full list of ancestors. + commit?: string; + history?: Record; +} + +interface FakeRecord { + logs: string[]; + outputs: Record; + commands: string[]; + sleeps: number; +} + +const unreadable = (url: string) => new Error(`Could not read ${url}: fetch failed (ENOTFOUND).`); +const distTagsUrl = 'https://registry.npmjs.org/-/package/@angular/fire/dist-tags'; + +/** A publish job with no network, no git, no npm and a clock that only `sleep` moves. */ +function fakeActions({ version, distTagReads, gitTags, npmFails = false, commit = '', history = {} }: FakeActionsOptions) { + let clock = 0; + const reads = [...distTagReads]; + const record: FakeRecord = { logs: [], outputs: {}, commands: [], sleeps: 0 }; + const actions = { + version: () => version, + fetchDistTags: async () => { + const read = reads.length > 1 ? reads.shift() : reads[0]; + if (read instanceof Error) { + throw read; + } + return read; + }, + gitTags: () => { + if (!gitTags) { + throw new Error('This step must not read the repository\'s tags.'); + } + return gitTags; + }, + commit: () => commit, + cloneHistory: () => undefined, + resolveCommit: (abbreviation: string) => { + const matches = Object.keys(history).filter(sha => sha.startsWith(abbreviation)); + if (matches.length !== 1) { + throw new Error('fatal: Needed a single revision'); + } + return matches[0]; + }, + isAncestor: (ancestor: string, descendant: string) => ancestor === descendant || (history[descendant] ?? []).includes(ancestor), + run: (command: string, args: string[]) => { + record.commands.push([command, ...args].join(' ')); + if (npmFails) { + throw new Error('npm error code E400'); + } + }, + sleep: async (seconds: number) => { + clock += seconds * 1000; + record.sleeps++; + }, + now: () => clock, + setOutput: (name: string, value: string) => { + record.outputs[name] = value; + }, + log: (message: string) => { + record.logs.push(message); + }, + }; + return { actions, record }; +} + +const tagsAfter21 = ['20.0.3', '20.1.0', '21.0.0-rc.1', '21.0.0']; +const moveNextCommand = 'npm dist-tag add @angular/fire@21.0.0 next --registry https://wombat-dressing-room.appspot.com'; + +describe('publish-job.js canary-check', () => { + // main: aaaaaaa1, then bbbbbbb2, then ccccccc3. ddddddd4 is on no line of main's history. + const history = { aaaaaaa1: [], bbbbbbb2: ['aaaaaaa1'], ccccccc3: ['aaaaaaa1', 'bbbbbbb2'], ddddddd4: [] }; + const canaryOf = (sha: string) => `21.0.1-canary.20261001000000.sha-${sha.slice(0, 7)}`; + const check = (commit: string, npmCanary: string) => + fakeActions({ version: canaryOf(commit), distTagReads: [{ canary: npmCanary }], commit, history }); + + it('publishes a canary of a later commit', async () => { + const { actions, record } = check('ccccccc3', canaryOf('bbbbbbb2')); + expect(await publishJob.runStep('canary-check', [], actions)).toBe(0); + expect([record.outputs, record.logs]).toEqual([{}, []]); + }); + + it('skips a canary of an earlier commit', async () => { + const { actions, record } = check('aaaaaaa1', canaryOf('bbbbbbb2')); + expect(await publishJob.runStep('canary-check', [], actions)).toBe(0); + expect(record.outputs).toEqual({ skip: 'true' }); + expect(record.logs).toEqual([`::warning::Not publishing ${canaryOf('aaaaaaa1')}, because the canary on npm, ${canaryOf('bbbbbbb2')}, is from a later commit on main.`]); + }); + + it('skips the canary already on npm, and publishes another version of the same commit', async () => { + const same = check('bbbbbbb2', canaryOf('bbbbbbb2')); + expect(await publishJob.runStep('canary-check', [], same.actions)).toBe(0); + expect(same.record.outputs).toEqual({ skip: 'true' }); + expect(same.record.logs[0]).toMatch(/^::notice::Not publishing .* because it is already the canary on npm\.$/); + const renamed = check('bbbbbbb2', '21.0.0-canary.bbbbbbb'); + expect(await publishJob.runStep('canary-check', [], renamed.actions)).toBe(0); + expect(renamed.record.outputs).toEqual({}); + }); + + it('fails when the canary on npm is not a commit on main\'s line', async () => { + const unrelated = check('ccccccc3', canaryOf('ddddddd4')); + expect(await publishJob.runStep('canary-check', [], unrelated.actions)).toBe(1); + expect(unrelated.record.logs[0]).toMatch(/^::error::Not publishing .* are not on the same line of history\. One of them is not on main\. Every canary publish fails/); + const unknown = check('ccccccc3', canaryOf('eeeeeee5')); + expect(await publishJob.runStep('canary-check', [], unknown.actions)).toBe(1); + expect(unknown.record.logs[0]).toMatch(/^::error::Could not match the canary on npm, .*sha-eeeeeee, to a single commit in this repository\./); + }); +}); + +describe('publish-job.js release-tag', () => { + + it('publishes a new major to latest', async () => { + const { actions, record } = fakeActions({ version: '21.0.0', distTagReads: [{ latest: '20.1.0', next: '21.0.0-rc.1' }], gitTags: tagsAfter21 }); + expect(await publishJob.runStep('release-tag', [], actions)).toBe(0); + expect(record.outputs).toEqual({ tag: 'latest' }); + expect(record.logs).toEqual(['Publishing 21.0.0 under latest.']); + }); + + it('publishes an older major to its lts tag and says what outranks it', async () => { + const { actions, record } = fakeActions({ version: '20.1.1', distTagReads: [{ latest: '21.0.0' }], gitTags: [...tagsAfter21, '20.1.1'] }); + expect(await publishJob.runStep('release-tag', [], actions)).toBe(0); + expect(record.outputs).toEqual({ tag: 'v20-lts' }); + expect(record.logs).toEqual(['::warning::20.1.1 goes to v20-lts, not latest, because it does not rank above 21.0.0.', 'Publishing 20.1.1 under v20-lts.']); + }); + + it('publishes a prerelease to next', async () => { + const { actions, record } = fakeActions({ version: '21.0.0-rc.2', distTagReads: [{ latest: '20.1.0', next: '21.0.0-rc.1' }], gitTags: tagsAfter21 }); + expect(await publishJob.runStep('release-tag', [], actions)).toBe(0); + expect(record.outputs).toEqual({ tag: 'next' }); + }); + + it('skips publishing on a re-run of a release npm already lists', async () => { + const { actions, record } = fakeActions({ version: '21.0.0', distTagReads: [{ latest: '21.0.0', next: '21.0.0-rc.1' }] }); + expect(await publishJob.runStep('release-tag', [], actions)).toBe(0); + expect(record.outputs).toEqual({ tag: 'latest', published: 'true' }); + expect(record.logs).toEqual(['npm already lists 21.0.0 as latest, so this run skips publishing it.']); + const lts = fakeActions({ version: '20.1.1', distTagReads: [{ latest: '21.0.0', 'v20-lts': '20.1.1' }] }); + expect(await publishJob.runStep('release-tag', [], lts.actions)).toBe(0); + expect(lts.record.outputs).toEqual({ tag: 'v20-lts', published: 'true' }); + const candidate = fakeActions({ version: '21.1.0-rc.0', distTagReads: [{ latest: '21.0.0', next: '21.1.0-rc.0' }] }); + expect(await publishJob.runStep('release-tag', [], candidate.actions)).toBe(0); + expect(candidate.record.outputs).toEqual({ tag: 'next', published: 'true' }); + }); + + it('fails without choosing a tag when the release must not publish', async () => { + const { actions, record } = fakeActions({ + version: '20.0.4', distTagReads: [{ latest: '21.0.0', 'v20-lts': '20.1.1' }], gitTags: [...tagsAfter21, '20.1.1', '20.0.4'], + }); + expect(await publishJob.runStep('release-tag', [], actions)).toBe(1); + expect(record.outputs).toEqual({}); + expect(record.logs).toEqual(['::error::Not publishing 20.0.4 under v20-lts, because it does not rank above 20.1.1, 20.1.0.']); + }); + + it('fails when npm cannot be read', async () => { + const { actions, record } = fakeActions({ version: '21.0.0', distTagReads: [unreadable(distTagsUrl)], gitTags: tagsAfter21 }); + expect(await publishJob.runStep('release-tag', [], actions)).toBe(1); + expect(record.logs).toEqual([`::error::Could not read ${distTagsUrl}: fetch failed (ENOTFOUND).`]); + }); +}); + +describe('publish-job.js wait', () => { + const stale = { latest: '20.1.0', canary: '21.0.1-canary.1' }; + const listed = { latest: '21.0.0', canary: '21.0.1-canary.1' }; + + it('waits until npm lists the version under its tag', async () => { + const { actions, record } = fakeActions({ version: '21.0.0', distTagReads: [stale, unreadable(distTagsUrl), listed] }); + expect(await publishJob.runStep('wait', ['latest'], actions)).toBe(0); + expect(record.sleeps).toBe(2); + expect(record.outputs).toEqual({ listed: 'true' }); + expect(record.logs).toEqual(['npm lists 21.0.0 as latest, 30s after publishing.']); + }); + + it('fails after 20 minutes for latest, printing the command that moves next', async () => { + const { actions, record } = fakeActions({ version: '21.0.0', distTagReads: [stale] }); + expect(await publishJob.runStep('wait', ['latest'], actions)).toBe(1); + expect(record.sleeps).toBe(80); + expect(record.outputs).toEqual({}); + expect(record.logs[0]).toMatch(/^::warning::npm does not list 21\.0\.0 as latest 20 minutes after publishing/); + expect(record.logs[1]).toBe('::error::next was not moved to 21.0.0. Once npm lists 21.0.0, publish rights are required to run: npm dist-tag add @angular/fire@21.0.0 next'); + }); + + it('only warns after 20 minutes for any other tag', async () => { + const { actions, record } = fakeActions({ version: '21.0.1-canary.2', distTagReads: [stale] }); + expect(await publishJob.runStep('wait', ['canary'], actions)).toBe(0); + expect(record.outputs).toEqual({}); + expect(record.logs.length).toBe(1); + expect(record.logs[0]).toMatch(/^::warning::/); + }); +}); + +describe('publish-job.js move-next', () => { + + it('moves next up to the release', async () => { + const { actions, record } = fakeActions({ version: '21.0.0', distTagReads: [{ latest: '21.0.0', next: '21.0.0-rc.1' }] }); + expect(await publishJob.runStep('move-next', [], actions)).toBe(0); + expect(record.commands).toEqual([moveNextCommand]); + }); + + it('leaves a higher next alone', async () => { + const { actions, record } = fakeActions({ version: '21.0.1', distTagReads: [{ latest: '21.0.1', next: '21.1.0-rc.0' }] }); + expect(await publishJob.runStep('move-next', [], actions)).toBe(0); + expect(record.commands).toEqual([]); + expect(record.logs).toEqual(['Not moving next, because it is 21.1.0-rc.0.']); + }); + + it('prints the command that moves next on every failure', async () => { + const fix = 'Publish rights are required to move it with: npm dist-tag add @angular/fire@21.0.0 next'; + const failures: [FakeActionsOptions, string][] = [ + [{ version: '21.0.0', distTagReads: [unreadable(distTagsUrl)] }, `::error::Could not read ${distTagsUrl}: fetch failed (ENOTFOUND). So next was not checked. ${fix}`], + [{ version: '21.0.0', distTagReads: [{ latest: '21.0.0', next: '21.0.0-rc.1' }], npmFails: true }, `::error::21.0.0 is published, but next was not moved to it. ${fix}`], + ]; + for (const [options, error] of failures) { + const { actions, record } = fakeActions(options); + expect(await publishJob.runStep('move-next', [], actions)).toBe(1); + expect(record.logs).toEqual([error]); + } + }); +}); + + +describe('publish-job.js actions', () => { + const git = (cwd: string, ...args: string[]) => execFileSync('git', ['-c', 'user.name=spec', '-c', 'user.email=spec@example.com', ...args], { cwd, encoding: 'utf8' }).trim(); + const envKeys = ['GITHUB_SERVER_URL', 'GITHUB_REPOSITORY', 'GITHUB_OUTPUT']; + const savedEnv = envKeys.map(key => [key, process.env[key]]); + const savedCwd = process.cwd(); + let workDir: string; + let commits: string[]; + + // A repository named `origin` with three commits on one line, tagged 21.0.0 and 21.0.1, plus an unrelated commit. + beforeEach(() => { + workDir = mkdtempSync(join(tmpdir(), 'publish-job-')); + const origin = join(workDir, 'origin'); + git(workDir, 'init', '--quiet', 'origin'); + commits = ['one', 'two', 'three'].map(message => { + git(origin, 'commit', '--quiet', '--allow-empty', '-m', message); + return git(origin, 'rev-parse', 'HEAD'); + }); + git(origin, 'tag', '21.0.0', commits[0]); + git(origin, 'tag', '-a', '21.0.1', '-m', 'annotated', commits[1]); + git(origin, 'checkout', '--quiet', '--orphan', 'unrelated'); + git(origin, 'commit', '--quiet', '--allow-empty', '-m', 'unrelated'); + commits.push(git(origin, 'rev-parse', 'HEAD')); + // A file:// address makes git honor --filter, as it does for the real repository. + process.env.GITHUB_SERVER_URL = `file://${workDir}`; + process.env.GITHUB_REPOSITORY = 'origin'; + process.env.GITHUB_OUTPUT = join(workDir, 'output'); + process.chdir(workDir); + }); + + afterEach(() => { + process.chdir(savedCwd); + for (const [key, value] of savedEnv) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + fsExtra.removeSync(workDir); + }); + + it('lists the repository\'s tag names', () => { + expect(publishJob.actions.gitTags().sort()).toEqual(['21.0.0', '21.0.1']); + }); + + it('resolves abbreviations and orders commits in the cloned history', () => { + publishJob.actions.cloneHistory(); + const [one, two, three, unrelated] = commits; + expect(publishJob.actions.resolveCommit(three.slice(0, 7))).toBe(three); + expect(() => publishJob.actions.resolveCommit('0000000')).toThrow(); + expect([publishJob.actions.isAncestor(one, three), publishJob.actions.isAncestor(three, two), publishJob.actions.isAncestor(one, unrelated)]).toEqual([true, false, false]); + }); + + it('appends step outputs as name=value lines', () => { + writeFileSync(process.env.GITHUB_OUTPUT ?? '', ''); + publishJob.actions.setOutput('tag', 'latest'); + publishJob.actions.setOutput('listed', 'true'); + expect(readFileSync(process.env.GITHUB_OUTPUT ?? '', 'utf8')).toBe('tag=latest\nlisted=true\n'); + }); +}); diff --git a/tools/publish-job.js b/tools/publish-job.js new file mode 100644 index 000000000..92b928419 --- /dev/null +++ b/tools/publish-job.js @@ -0,0 +1,176 @@ +// No dependencies: the publish job runs this without installing node_modules. +const { execFileSync } = require('child_process'); +const { appendFileSync, readFileSync } = require('fs'); +const { highestReleaseAbove, movesNext, publishedUnder, releaseTag } = require('./release-tag.js'); + +const DIST_TAGS_URL = 'https://registry.npmjs.org/-/package/@angular/fire/dist-tags'; +const WOMBAT_URL = 'https://wombat-dressing-room.appspot.com'; +// npm lists a version only after its publish-time malware scan. +const WAIT_SECONDS = 1200; +const POLL_SECONDS = 15; + +/** Reads npm's dist-tags from the endpoint npm does not CDN-cache, unlike the package data `npm view` reads. */ +async function fetchDistTags() { + let lastError; + for (const pauseSeconds of [0, 2, 4]) { + await new Promise(resolve => setTimeout(resolve, pauseSeconds * 1000)); + try { + const response = await fetch(DIST_TAGS_URL, { signal: AbortSignal.timeout(30_000) }); + if (!response.ok) { + throw new Error(`it answered ${response.status}`); + } + return await response.json(); + } catch (error) { + lastError = error; + } + } + const reason = lastError.cause ? `${lastError.message} (${lastError.cause.code ?? lastError.cause.message})` : lastError.message; + throw new Error(`Could not read ${DIST_TAGS_URL}: ${reason}.`); +} + +const git = (args, options) => execFileSync('git', args, options); +const repositoryUrl = () => `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}`; + +/** The npm, git, file, clock and log calls the steps make on the GitHub Actions runner. */ +const actions = { + version: () => JSON.parse(readFileSync('dist/packages-dist/package.json', 'utf8')).version, + fetchDistTags, + gitTags: () => git(['ls-remote', '--tags', '--refs', repositoryUrl()], { encoding: 'utf8' }) + .split('\n').filter(line => line).map(line => line.replace(/.*refs\/tags\//, '')), + run: (command, args) => execFileSync(command, args, { stdio: 'inherit' }), + commit: () => process.env.GITHUB_SHA, + cloneHistory: () => git(['clone', '--quiet', '--bare', '--filter=tree:0', repositoryUrl(), 'history.git']), + resolveCommit: abbreviation => git(['-C', 'history.git', 'rev-parse', '--verify', '--quiet', `${abbreviation}^{commit}`], { encoding: 'utf8' }).trim(), + isAncestor: (ancestor, descendant) => { + try { + git(['-C', 'history.git', 'merge-base', '--is-ancestor', ancestor, descendant]); + return true; + } catch { + return false; + } + }, + sleep: seconds => new Promise(resolve => setTimeout(resolve, seconds * 1000)), + now: () => Date.now(), + setOutput: (name, value) => appendFileSync(process.env.GITHUB_OUTPUT, `${name}=${value}\n`), + log: message => console.log(message), +}; + +/** + * Sets the step output `skip` for a canary whose commit is not after the commit of the canary on + * npm, since `npm publish` always moves the dist-tag. Ordered by position on main, not by version, + * which can be higher for an older commit. + */ +async function checkCanary(actions) { + const version = actions.version(); + const npmCanary = (await actions.fetchDistTags()).canary; + actions.cloneHistory(); + const commit = actions.commit(); + const howToFix = 'Every canary publish fails until the canary dist-tag points at a build of a commit on main. Publish rights are required to fix it with: npm dist-tag add @angular/fire@ canary'; + + let npmCanaryCommit; + try { npmCanaryCommit = actions.resolveCommit(npmCanary.split(/[.-]/).pop()); } + catch { throw new Error(`Could not match the canary on npm, ${npmCanary}, to a single commit in this repository. ${howToFix}`); } + + if (npmCanaryCommit === commit) { + if (version === npmCanary) { + actions.log(`::notice::Not publishing ${version}, because it is already the canary on npm.`); + actions.setOutput('skip', 'true'); + } + } else if (actions.isAncestor(commit, npmCanaryCommit)) { + actions.log(`::warning::Not publishing ${version}, because the canary on npm, ${npmCanary}, is from a later commit on main.`); + actions.setOutput('skip', 'true'); + } else if (!actions.isAncestor(npmCanaryCommit, commit)) { + throw new Error(`Not publishing ${version}, because its commit and the commit of the canary on npm, ${npmCanary}, are not on the same line of history. One of them is not on main. ${howToFix}`); + } +} + +/** + * Chooses the dist-tag a tagged release publishes under and sets the step output `tag`. On a re-run + * of a release npm already lists, also sets `published`, so the job skips `npm publish` and goes on + * to the steps after it. + */ +async function chooseReleaseTag(actions) { + const version = actions.version(); + const distTags = await actions.fetchDistTags(); + const alreadyUnder = publishedUnder(version, distTags); + if (alreadyUnder) { + actions.log(`npm already lists ${version} as ${alreadyUnder}, so this run skips publishing it.`); + actions.setOutput('tag', alreadyUnder); + actions.setOutput('published', 'true'); + return; + } + // A higher release can be tagged and still be in npm's publish-time malware scan, so git tags count too. + const gitTags = actions.gitTags(); + const tag = releaseTag(version, distTags, gitTags); + if (tag.endsWith('-lts')) { + actions.log(`::warning::${version} goes to ${tag}, not latest, because it does not rank above ${highestReleaseAbove(version, distTags, gitTags)}.`); + } + actions.log(`Publishing ${version} under ${tag}.`); + actions.setOutput('tag', tag); +} + +/** + * Waits until npm lists the published version under `tag`, holding the publish queue so the next + * publish reads it, and sets the step output `listed`. Fails on timeout only for `latest`, since + * `next` then cannot be moved. + */ +async function waitUntilListed(actions, tag) { + const version = actions.version(); + const start = actions.now(); + while (actions.now() - start < WAIT_SECONDS * 1000) { + const distTags = await actions.fetchDistTags().catch(() => ({})); + if (distTags[tag] === version) { + actions.log(`npm lists ${version} as ${tag}, ${Math.round((actions.now() - start) / 1000)}s after publishing.`); + actions.setOutput('listed', 'true'); + return; + } + await actions.sleep(POLL_SECONDS); + } + actions.log(`::warning::npm does not list ${version} as ${tag} ${WAIT_SECONDS / 60} minutes after publishing, so the next publish may read the previous ${tag}.`); + if (tag === 'latest') { + throw new Error(`next was not moved to ${version}. Once npm lists ${version}, publish rights are required to run: npm dist-tag add @angular/fire@${version} next`); + } +} + +/** Moves `next` up to a release npm now lists as `latest`, unless `next` is already higher. */ +async function moveNext(actions) { + const version = actions.version(); + const fix = `Publish rights are required to move it with: npm dist-tag add @angular/fire@${version} next`; + + let distTags; + try { distTags = await actions.fetchDistTags(); } + catch (error) { throw new Error(`${error.message} So next was not checked. ${fix}`); } + + if (!movesNext(version, distTags)) { + actions.log(`Not moving next, because it is ${distTags.next}.`); + return; + } + + try { actions.run('npm', ['dist-tag', 'add', `@angular/fire@${version}`, 'next', '--registry', WOMBAT_URL]); } + catch { throw new Error(`${version} is published, but next was not moved to it. ${fix}`); } +} + +const steps = { + 'canary-check': actions => checkCanary(actions), + 'release-tag': actions => chooseReleaseTag(actions), + 'wait': (actions, tag) => waitUntilListed(actions, tag), + 'move-next': actions => moveNext(actions), +}; + +/** Runs one step, turning a thrown error into an `::error::` line and exit code 1. */ +async function runStep(name, args, actions) { + try { + await steps[name](actions, ...args); + return 0; + } catch (error) { + actions.log(`::error::${error.message}`); + return 1; + } +} + +if (require.main === module) { + const [name, ...args] = process.argv.slice(2); + runStep(name, args, actions).then(code => process.exit(code)); +} + +module.exports = { actions, runStep }; diff --git a/tools/release-tag.jasmine.ts b/tools/release-tag.jasmine.ts new file mode 100644 index 000000000..e31738594 --- /dev/null +++ b/tools/release-tag.jasmine.ts @@ -0,0 +1,131 @@ +import { gt as semverGt } from 'semver'; +import { highestReleaseAbove, movesNext, publishedUnder, releasesAbove, releaseTag } from './release-tag.js'; +import 'jasmine'; + +/* npm's dist-tags and the repository's stable tags once 21.0.0 ships. */ +const after21 = { latest: '21.0.0', next: '21.0.0', canary: '21.0.1-canary.20261005034752.sha-bc3fdad' }; +const tagsAfter21 = ['5.1', 'v5.2.0', '20.0.3', '20.1.0', '21.0.0-rc.1', '21.0.0']; + +describe('releaseTag', () => { + + it('publishes a new major to latest', () => { + const today = { latest: '20.1.0', next: '21.0.0-rc.1' }; + expect(releaseTag('21.0.0', today, ['20.0.3', '20.1.0', '21.0.0-rc.1', '21.0.0'])).toBe('latest'); + }); + + it('publishes a patch, minor or major above latest to latest', () => { + expect(releaseTag('21.0.1', after21, [...tagsAfter21, '21.0.1'])).toBe('latest'); + expect(releaseTag('21.1.0', after21, [...tagsAfter21, '21.1.0'])).toBe('latest'); + expect(releaseTag('22.0.0', after21, [...tagsAfter21, '22.0.0'])).toBe('latest'); + }); + + it('compares each number, not the version text', () => { + const at21dot9 = { latest: '21.9.0' }; + expect(releaseTag('21.10.0', at21dot9, ['21.9.0', '21.10.0'])).toBe('latest'); + expect(() => releaseTag('21.9.1', { latest: '21.10.0' }, ['21.10.0', '21.9.1'])).toThrowError(/does not rank above 21\.10\.0\.$/); + }); + + it('publishes an older major to its lts tag', () => { + expect(releaseTag('20.1.1', after21, [...tagsAfter21, '20.1.1'])).toBe('v20-lts'); + }); + + it('publishes to the lts tag while a higher tagged release is still in npm\'s scan', () => { + const stillScanning = { latest: '20.1.0', next: '21.0.0-rc.1' }; + expect(releaseTag('20.1.1', stillScanning, [...tagsAfter21, '20.1.1'])).toBe('v20-lts'); + }); + + it('publishes a later patch to an existing lts tag', () => { + const withLts = { ...after21, 'v20-lts': '20.1.1' }; + expect(releaseTag('20.1.2', withLts, [...tagsAfter21, '20.1.1', '20.1.2'])).toBe('v20-lts'); + }); + + it('ignores prerelease tags above the release', () => { + expect(releaseTag('21.0.1', after21, [...tagsAfter21, '21.1.0-rc.0', '22.0.0-next.0', '21.0.1'])).toBe('latest'); + }); + + it('refuses a release below the current lts version', () => { + const withLts = { ...after21, 'v20-lts': '20.1.1' }; + expect(() => releaseTag('20.0.4', withLts, [...tagsAfter21, '20.1.1', '20.0.4'])).toThrowError(/does not rank above 20\.1\.1/); + expect(() => releaseTag('20.1.1', withLts, [...tagsAfter21, '20.1.1'])).toThrowError(/does not rank above 20\.1\.1/); + }); + + it('refuses the first lts release of a major below another release of that major', () => { + expect(() => releaseTag('20.0.4', after21, [...tagsAfter21, '20.0.4'])).toThrowError('Not publishing 20.0.4 under v20-lts, because it does not rank above 20.1.0.'); + }); + + it('publishes every prerelease to next', () => { + expect(releaseTag('21.1.0-rc.0', after21, tagsAfter21)).toBe('next'); + expect(releaseTag('20.2.0-rc.0', after21, tagsAfter21)).toBe('next'); + expect(releaseTag('22.0.0-next.0', after21, tagsAfter21)).toBe('next'); + expect(releaseTag('22.0.0-alpha.1', after21, tagsAfter21)).toBe('next'); + }); + + it('refuses versions it cannot read', () => { + expect(() => releaseTag('21.0', after21, tagsAfter21)).toThrowError(/not a semver version/); + expect(() => releaseTag('21.0.0-', after21, tagsAfter21)).toThrowError(/not a semver version/); + }); + + it('ranks stable versions exactly as semver does', () => { + const others = ['20.9.9', '21.0.0-rc.1', '21.0.0', '21.0.1-rc.0', '21.0.1', '21.0.2', '21.1.0-rc.0', '21.1.0', '21.10.0', '22.0.0-canary.1']; + for (const version of ['21.0.0', '21.0.1', '21.1.0', '21.9.0', '21.10.0']) { + for (const other of others) { + const expected = semverGt(version, other) ? 'latest' : 'v21-lts'; + expect(`${version} over ${other}: ${releaseTag(version, { latest: other }, [version])}`) + .toBe(`${version} over ${other}: ${expected}`); + } + } + }); +}); + +describe('movesNext', () => { + + // By the time this runs, npm's `latest` is already the release. + it('moves next up to a stable release', () => { + expect(movesNext('21.0.0', { latest: '21.0.0', next: '21.0.0-rc.1' })).toBeTrue(); + expect(movesNext('21.0.1', { latest: '21.0.1', next: '21.0.0' })).toBeTrue(); + expect(movesNext('21.0.1', { latest: '21.0.1', next: '21.0.1-rc.0' })).toBeTrue(); + }); + + it('moves next when npm has none', () => { + expect(movesNext('21.0.0', { latest: '21.0.0' })).toBeTrue(); + }); + + it('leaves next on a higher version', () => { + expect(movesNext('21.0.1', { latest: '21.0.1', next: '21.1.0-rc.0' })).toBeFalse(); + expect(movesNext('21.0.0', { latest: '21.0.0', next: '21.0.0' })).toBeFalse(); + }); + + it('never moves next to a prerelease', () => { + expect(movesNext('21.1.0-rc.0', { latest: '21.0.0', next: '21.0.0' })).toBeFalse(); + }); +}); + +describe('publishedUnder', () => { + + it('is undefined for a version npm does not list', () => { + expect(publishedUnder('21.0.1', after21)).toBeUndefined(); + }); + + it('names the dist-tag that holds the version, preferring latest', () => { + expect(publishedUnder('21.0.0', { next: '21.0.0', latest: '21.0.0' })).toBe('latest'); + expect(publishedUnder('20.1.1', { latest: '21.0.0', 'v20-lts': '20.1.1' })).toBe('v20-lts'); + }); +}); + +describe('highestReleaseAbove', () => { + + it('names only the highest release the version does not rank above', () => { + const tags = [...tagsAfter21, '22.0.0', '22.6.3', '22.1.0', '21.0.1']; + expect(highestReleaseAbove('21.0.1', { latest: '22.0.0' }, tags)).toBe('22.6.3'); + expect(highestReleaseAbove('20.1.1', { latest: '20.1.0' }, [...tagsAfter21, '21.1.0', '20.1.1'])).toBe('21.1.0'); + }); +}); + +describe('releasesAbove', () => { + + it('lists npm\'s latest and the stable git tags the release does not rank above, once each', () => { + expect(releasesAbove('20.1.1', after21, [...tagsAfter21, '20.1.1'])).toEqual(['21.0.0']); + expect(releasesAbove('21.0.2', after21, [...tagsAfter21, '21.1.0', '21.0.2'])).toEqual(['21.1.0']); + expect(releasesAbove('21.0.1', after21, [...tagsAfter21, '21.1.0-rc.0', '21.0.1'])).toEqual([]); + }); +}); diff --git a/tools/release-tag.js b/tools/release-tag.js new file mode 100644 index 000000000..94eb46d98 --- /dev/null +++ b/tools/release-tag.js @@ -0,0 +1,82 @@ +// No dependencies: the publish job runs this without installing node_modules. +const STABLE = /^v?(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)$/; +const ANY = /^v?(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(-[0-9A-Za-z.-]+)?(\+[0-9A-Za-z.-]+)?$/; + +/** Splits a version or tag into its three numbers and whether it is a prerelease. Throws if it is not semver. */ +function parse(version) { + const match = ANY.exec(version); + if (!match) { + throw new Error(`${version} is not a semver version.`); + } + return { numbers: match.slice(1, 4).map(Number), isPrerelease: !!match[4] }; +} + +/** Whether the stable `version` ranks above `other` in semver order. Only exact for a stable `version`. */ +function isAbove(version, other) { + const [mine, theirs] = [parse(version), parse(other)]; + for (let i = 0; i < 3; i++) { + if (mine.numbers[i] !== theirs.numbers[i]) { + return mine.numbers[i] > theirs.numbers[i]; + } + } + return theirs.isPrerelease; +} + +/** Whether `tag` and `version` have the same three numbers, ignoring a `v` prefix and any prerelease part. */ +const sameRelease = (tag, version) => parse(tag).numbers.join('.') === parse(version).numbers.join('.'); + +/** + * Picks the npm dist-tag a tagged release publishes under. + * + * A stable version goes to `latest` only if it ranks above both npm's `latest` and every other + * stable git tag, since a higher release can be tagged and still be in npm's publish-time malware scan. + * Any other stable version goes to `v-lts`, and must rank above that tag's current version. + * + * @param {string} version The version being published. + * @param {Record} distTags npm's current dist-tags. + * @param {string[]} gitTags Every tag name in the repository. + * @returns {string} The dist-tag. Throws if the version must not be published. + */ +function releaseTag(version, distTags, gitTags) { + if (!STABLE.test(version)) { + parse(version); + return 'next'; + } + if (releasesAbove(version, distTags, gitTags).length === 0) { + return 'latest'; + } + const major = parse(version).numbers[0]; + const ltsTag = `v${major}-lts`; + const sameMajorReleases = gitTags.filter(tag => STABLE.test(tag) && !sameRelease(tag, version) && parse(tag).numbers[0] === major); + const blocking = [distTags[ltsTag], ...sameMajorReleases].filter(release => release && !isAbove(version, release)); + if (blocking.length) { + throw new Error(`Not publishing ${version} under ${ltsTag}, because it does not rank above ${[...new Set(blocking)].join(', ')}.`); + } + return ltsTag; +} + +/** + * The dist-tag npm already lists `version` under, preferring `latest`, which means this run is a + * re-run of a published release. + */ +function publishedUnder(version, distTags) { + return distTags.latest === version ? 'latest' : Object.keys(distTags).find(tag => distTags[tag] === version); +} + +/** The highest of npm's `latest` and the other stable git tags that `version` does not rank above. */ +function highestReleaseAbove(version, distTags, gitTags) { + return releasesAbove(version, distTags, gitTags).reduce((highest, release) => (isAbove(release, highest) ? release : highest)); +} + +/** npm's `latest` and the other stable git tags that `version` does not rank above. */ +function releasesAbove(version, distTags, gitTags) { + const otherReleases = gitTags.filter(tag => STABLE.test(tag) && !sameRelease(tag, version)); + return [...new Set([distTags.latest, ...otherReleases])].filter(release => !isAbove(version, release)); +} + +/** Whether a published release should become `next`: it is stable and above npm's `next`. */ +function movesNext(version, distTags) { + return STABLE.test(version) && (!distTags.next || isAbove(version, distTags.next)); +} + +module.exports = { highestReleaseAbove, movesNext, publishedUnder, releasesAbove, releaseTag }; From 4e2459ae8d110e512ddf719d536a7f4fafd01231 Mon Sep 17 00:00:00 2001 From: Armando Navarro Date: Fri, 9 Oct 2026 14:07:47 -0700 Subject: [PATCH 09/11] build: compile the tools specs in the jasmine build The publish job's specs (tools/publish-job.jasmine.ts and tools/release-tag.jasmine.ts) live under tools/. On main, #3780 added this include, and #3780 itself stays off 20.1.x, so without it build:jasmine never compiles those 39 specs and test:node skips them without failing. --- tsconfig.jasmine.json | 1 + 1 file changed, 1 insertion(+) diff --git a/tsconfig.jasmine.json b/tsconfig.jasmine.json index 49cf3de0a..d3d584717 100644 --- a/tsconfig.jasmine.json +++ b/tsconfig.jasmine.json @@ -14,6 +14,7 @@ }, "include": [ "tools/jasmine.ts", + "tools/**/*.jasmine.ts", "src/**/*.jasmine.ts", // Not sure what is wrong here, but since upgrading karma it's fallen apart // "src/**/*.spec.ts", From 0fa01eae644cb09bc524b6b66da0b796d02624e8 Mon Sep 17 00:00:00 2001 From: Armando Navarro Date: Fri, 9 Oct 2026 11:58:57 -0700 Subject: [PATCH 10/11] fix(storage): give the compat fromTask an explicit return type (#3796) fromTask declared no return type, so the published typings import whatever path the typings bundler picks for the inferred firebase type. In 20.x that is 'firebase/compat', which firebase's exports map does not list. Apps using moduleResolution "bundler", the setting the @angular/build migration moves apps to, fail with TS2307 in @angular/fire/compat/storage. 21.0.0-rc.1 happens to emit 'firebase/compat/app' and compiles, but ng-packagr 22.2's new typings bundler cannot parse the inferred type and fails the library build. Declaring Observable removes the inferred import. The type is unchanged: UploadTaskSnapshot is the compat alias for firebase.storage.UploadTaskSnapshot. Also drop a comment explaining a firebase import that #3421 removed in 2023 as unused. The import only existed to steer these typings. Fixes #3677 (cherry picked from commit 4c5ba7e2ed92e48f5ef645c6af5e5bbfdb93d97b) --- src/compat/storage/observable/fromTask.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/compat/storage/observable/fromTask.ts b/src/compat/storage/observable/fromTask.ts index 85f8a34e5..72905b003 100644 --- a/src/compat/storage/observable/fromTask.ts +++ b/src/compat/storage/observable/fromTask.ts @@ -2,12 +2,9 @@ import { Observable } from 'rxjs'; import { debounceTime } from 'rxjs/operators'; import { UploadTask, UploadTaskSnapshot } from '../interfaces'; -// need to import, else the types become import('firebase/compat/app').default.storage.UploadTask -// and it no longer works w/Firebase v7 - // Things aren't working great, I'm having to put in a lot of work-arounds for what // appear to be Firebase JS SDK bugs https://github.com/firebase/firebase-js-sdk/issues/4158 -export function fromTask(task: UploadTask) { +export function fromTask(task: UploadTask): Observable { return new Observable(subscriber => { const progress = (snap: UploadTaskSnapshot) => subscriber.next(snap); const error = e => subscriber.error(e); From 69d94e60204247cce4f264ce99ae9d6f6cc7a41e Mon Sep 17 00:00:00 2001 From: Armando Navarro Date: Sun, 27 Sep 2026 16:59:54 -0700 Subject: [PATCH 11/11] fix(auth): stop beforeAuthStateChanged from holding the app unstable (#3770) AngularFire wrapped beforeAuthStateChanged so that registering the hook added a pending task, cleared only when the callback first runs. Firebase runs that callback only on a sign-in or sign-out, so for a visitor who does neither the app never became stable. Registered on the server, it failed ng build during route extraction and left server-rendered requests without a response. This restores the blockUntilFirst: false override from #3590, which #3613 dropped without comment while adding log-level overrides next to it. The callback still runs inside Angular's zone and injection context, and its returned promise still reaches Firebase, so a rejection still cancels the sign-in. A call outside an injection context now logs its per-call warning only at the verbose level, as onMessage does. Fixes #3748 docs(auth): scope the beforeAuthStateChanged note to rc.1 and earlier Merging this change closes #3748, so the section's present-tense note would point at a closed issue. Also removed the false claim that the @angular/fire/auth import makes ng build hang: the guide registers the hook only in the browser, so its own build succeeds. The docs/auth.md change is left out, since it edits a section of the guide that 20.1.x does not have. (cherry picked from commit f18297253b3d095844e4c673da63ec3cb5f87abd) --- src/auth/firebase.ts | 2 +- tools/build.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/auth/firebase.ts b/src/auth/firebase.ts index 743ca90d8..5d359e54d 100644 --- a/src/auth/firebase.ts +++ b/src/auth/firebase.ts @@ -58,7 +58,7 @@ import { } from 'firebase/auth'; export const applyActionCode = ɵzoneWrap(_applyActionCode, true); -export const beforeAuthStateChanged = ɵzoneWrap(_beforeAuthStateChanged, true); +export const beforeAuthStateChanged = ɵzoneWrap(_beforeAuthStateChanged, false); export const checkActionCode = ɵzoneWrap(_checkActionCode, true); export const confirmPasswordReset = ɵzoneWrap(_confirmPasswordReset, true, 2); export const connectAuthEmulator = ɵzoneWrap(_connectAuthEmulator, true); diff --git a/tools/build.ts b/tools/build.ts index d4daf50a6..a00606ad0 100644 --- a/tools/build.ts +++ b/tools/build.ts @@ -139,6 +139,8 @@ ${exportedZoneWrappedFns} indexedDBLocalPersistence: null, prodErrorMap: null, multiFactor: null, + // Its callback fires only on a sign-in or sign-out, so blocking would keep `ApplicationRef.isStable` false. + beforeAuthStateChanged: { blockUntilFirst: false }, linkWithCredential: { logLevel: LogLevel.VERBOSE }, linkWithPhoneNumber: { logLevel: LogLevel.VERBOSE }, linkWithPopup: { logLevel: LogLevel.VERBOSE },