From 14a333bdbf5d09b9dffc34339150fcb4497ec330 Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 07:42:52 -0700 Subject: [PATCH] fix(app-check): use reCAPTCHA Enterprise provider --- GITHUB_ISSUES.md | 10 ++- GITHUB_ISSUE_SLICES.md | 5 +- OPERATIONS_RUNBOOK.md | 17 +++++ SECURITY.md | 1 + .../firebase/FirebaseResources.test.ts | 69 ++++++++++++++++++- src/services/firebase/FirebaseResources.ts | 6 +- 6 files changed, 98 insertions(+), 10 deletions(-) diff --git a/GITHUB_ISSUES.md b/GITHUB_ISSUES.md index 67b2d4a0..fe4f2548 100644 --- a/GITHUB_ISSUES.md +++ b/GITHUB_ISSUES.md @@ -59,7 +59,7 @@ Every issue inherits `AGENTS.md` and the definition of done in `IMPLEMENTATION_P | 7 | AUTH-002 | Replace the legacy static-key membership synchronization endpoint | P0 | M | ready | AUTH-001 | | 8 | OAUTH-001 | Make Strava token lifecycle server-only, transactional, and auditable | P0 | M | represented by live [#88](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/88); proposed/unclaimed | SAFETY-001/#99; SEC-001 and AUTH-003 for A; staged deferral/cutover with ABUSE-001A for C | | 9 | AUTH-003 | Introduce scoped admin capabilities, MFA, and recent authentication | P1 | L | proposed | AUTH-001, AUTH-002, SEC-001 | -| 10 | ABUSE-001 | Enforce native App Check and privacy-preserving abuse limits | P0 | L | ready | CI-001 baseline | +| 10 | ABUSE-001 | Enforce native App Check and privacy-preserving abuse limits | P0 | L | partial: Enterprise browser source/tests tracked in [#159](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/159); provider config, native enforcement, rate limits, and live proof open | CI-001 baseline | | 11 | PAY-001 | Add strict request schemas and immutable monetary snapshots | P0 | L | partial: PAY-001A source/tests tracked in [#157](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/157); B/C/D and endpoint adoption open | ABUSE-001 interface agreed | | 12 | PROMO-001 | Disable unmodeled Stripe promotions until discounts are authoritative | P0 | S | source/tests complete under open GitHub issue #102; provider inventory and deployment remain owner action | PAY-001 for any future discount contract | | 13 | PAY-002 | Implement idempotent payment commands and explicit state machines | P0 | L | ready after PAY-001 | CONFIG-001, PAY-001 | @@ -488,9 +488,15 @@ Split claim migration, MFA/recent-auth, and UI route projection into separate PR ## ABUSE-001 — Enforce native App Check and privacy-preserving abuse limits **Labels:** `priority:P0`, `type:security`, `type:reliability`, `area:firebase`, `size:L`, `needs-external-config` -**Status:** Ready +**Status:** Partial. ABUSE-001A1 browser Enterprise source/tests are tracked in live [#159](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/159). Provider configuration, native callable enforcement, rate limits, alerts, deployment, and live proof remain open. **Depends on:** CI-001A recommended +### Current atomic boundary + +ABUSE-001A1 changes only the browser provider class from reCAPTCHA v3 to `ReCaptchaEnterpriseProvider`. It preserves local/test and initial capability-callback shutdown, requires the existing public build variable, keeps token auto-refresh, and retains fixed redacted diagnostics. Its tests use a synthetic key and no provider network call. + +This source slice does not configure an Enterprise key/domain policy, set a build variable, publish the website, exchange a live token, or enforce App Check on a Function. ABUSE-001A2 must separately inventory safe callables, preserve the DATA-001A/OAUTH-001C callback deferrals, add native runtime enforcement with emulator behavior, observe staged metrics, and prove missing/invalid/valid requests. #113/#133/#136/WEB-001 retain project, protected configuration, deployment, provider readback, and live verification. + ### Problem App Check is optional on the client and enforced only by a custom `ENFORCE_APP_CHECK === true` branch that is not configured in repository workflows. Rate limiting trusts the first `X-Forwarded-For`, stores raw IP/email, depends on an unverified TTL policy, and lacks per-business/user/concurrency/cost controls. diff --git a/GITHUB_ISSUE_SLICES.md b/GITHUB_ISSUE_SLICES.md index 5e370e86..9260a8a0 100644 --- a/GITHUB_ISSUE_SLICES.md +++ b/GITHUB_ISSUE_SLICES.md @@ -43,9 +43,10 @@ An agent-ready child has one observable outcome, one trust boundary, explicit de | OAUTH-001A | proposed; represented within canonical live [#88](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/88); SEC-001, AUTH-003B | Implement server-only, transaction/version-safe Strava token refresh so rotated refresh tokens cannot be lost. Do not publish a duplicate; claim/split only through #88. | Concurrent refresh/retry/conflict emulator tests. | | OAUTH-001B | owner_action; represented within canonical live [#88](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/88); OAUTH-001A | Minimize scopes, implement disconnect/provider revocation, record encryption decision, and add access/refresh/revoke audit. Do not publish a duplicate; claim/split only through #88. | Test-provider revoke/refresh evidence and approved threat decision. | | OAUTH-001C | proposed; represented within canonical live [#88](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/88); #99 | Create a server-safe Strava callback handoff that keeps `code`/`state` out of third parties while allowing App Check-protected `stravaExchangeCode`; retain state verification and server-only token exchange. Do not publish a duplicate; claim/split only through #88. | Encoded/case/trailing callback tests, wrong/replayed state denial, missing/invalid/valid App Check tests, and test-provider-only exchange evidence. | -| ABUSE-001A | ready; CI-001A | Replace custom fail-open App Check on an agreed sensitive-callable group with native runtime enforcement and explicit emulator behavior. Inventory every callable; defer `lookupRegistration`/`lookupOrder` to DATA-001A and `stravaExchangeCode` to OAUTH-001C within canonical tracker [#88](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/88) until safe handoffs merge. | Missing/invalid/valid tests plus staged metrics; source inventory test proves the three callback callables remain excluded until their dependency and regression evidence exist. | +| ABUSE-001A1 | browser Enterprise source/tests tracked in live [#159](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/159); CI-001A; not configured/published | Replace only the browser reCAPTCHA v3 provider class with the pinned SDK's Enterprise provider; preserve local/test/capability-callback shutdown, missing-key behavior, token refresh, and fixed diagnostics. | Synthetic-key focused/full/callback tests and source check prove Enterprise-only construction with no provider call; exact merge/CI evidence belongs to #159. | +| ABUSE-001A2 | proposed; A1, #113/#133 staged boundary | Replace custom fail-open App Check on an agreed sensitive-callable group with native runtime enforcement and explicit emulator behavior. Inventory every callable; defer `lookupRegistration`/`lookupOrder` to DATA-001A and `stravaExchangeCode` to OAUTH-001C within canonical tracker [#88](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/88) until safe handoffs merge. | Missing/invalid/valid tests plus staged metrics; source inventory proves the three callback callables remain excluded until their dependency and regression evidence exist. | | ABUSE-001B | proposed; PAY-001A | Replace raw IP/email rate keys with versioned HMAC, trusted address extraction, multi-scope transactional limits, rotation, and TTL-compatible records. | Spoof/boundary/concurrency/rotation tests; no raw identifiers. | -| ABUSE-001C | owner_action; A/B | Configure instance/concurrency/timeouts, budgets, TTL, and App Check/rate/cost alerts. | Synthetic alert and private cloud-setting review. | +| ABUSE-001C | owner_action; A2/B | Configure instance/concurrency/timeouts, budgets, TTL, and App Check/rate/cost alerts. | Synthetic alert and private cloud-setting review. | ## Configuration, validation, and payment foundation diff --git a/OPERATIONS_RUNBOOK.md b/OPERATIONS_RUNBOOK.md index 0d6ed991..e63efa9b 100644 --- a/OPERATIONS_RUNBOOK.md +++ b/OPERATIONS_RUNBOOK.md @@ -85,6 +85,23 @@ REACT_APP_STRAVA_CLIENT_ID The Firebase web configuration is public application configuration. It should still be environment-specific and paired with Auth restrictions, Firestore rules, and App Check. +### App Check release state — NOT AVAILABLE YET + +ABUSE-001A1 is tracked in live [#159](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/159). It changes only the browser source from the legacy reCAPTCHA v3 provider class to the Firebase SDK's Enterprise provider. It does not create or configure a key. + +Protected release `29254280177` stopped before checkout/build because `REACT_APP_RECAPTCHA_SITE_KEY` was absent. Nothing was published. Do not use a placeholder, reuse an unverified v3 key, expose a debug token, or weaken the release check. + +The safe sequence remains: + +1. Confirm the approved Firebase project and host inventory under #113. +2. Merge and verify #159 source/tests without a real key. +3. Have the named owner create the Enterprise site key and allowed-domain policy for the approved environment. +4. Bind website preparation to the approved protected Actions scope under #133/a dedicated CI child, then store only that public site key there. The current preparation job has no environment binding and can read only repository/organization variables; do not use that gap as a production shortcut. +5. Rehearse a synthetic staging page, observe App Check metrics, and prove expected token behavior before ABUSE-001A2 enables any callable enforcement. +6. Publish and verify the website, Firebase, provider, and live behavior as separate states under #136/WEB-001. + +A source merge, green CI run, or public-key variable alone does not prove Enterprise is configured or protecting a callable. + ### Server secrets Store server secrets in Google Secret Manager and bind only to functions that need them: diff --git a/SECURITY.md b/SECURITY.md index f068f6e9..18d97387 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -95,6 +95,7 @@ These entries are implementation evidence, not a production risk-acceptance deci | Risks | Source result | Remaining closure evidence | | --- | --- | --- | +| RISK-008 | ABUSE-001A1 is tracked in live [#159](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/159). The source-only slice replaces the browser reCAPTCHA v3 constructor with the pinned Firebase SDK's Enterprise provider and preserves local/test/capability-callback shutdown, missing-key behavior, token refresh, Analytics shutdown, and fixed redacted failure diagnostics. Synthetic tests use no real key or provider call. | No Enterprise key, allowed-domain policy, provider console setting, protected build variable, token exchange, staged metric, callable enforcement, website/Firebase publication, or live behavior is configured or proven. Release `29254280177` failed closed before build because the public site-key variable was absent. Complete #113/#133, ABUSE-001A2, #136/WEB-001, staged metrics, and provider readback before enforcement or a live claim. | | RISK-017 | PAY-001A is tracked in live [#157](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/157). The source slice adds dependency-free strict-root-object, root-budget, bounded string/array, integer-cents, fixed-currency, calendar-date, canonical-URL, conservative email, fixed-error, and closed-enum safe-log primitives. Synthetic hostile-input tests prove deep immutable copies and reject prototypes, accessors, and unknown configured root keys. The libraries contain no network, Firebase, Stripe, logger, clock, or random dependency and make no such call themselves; a supplied array item parser must be trusted, pure, and synchronous. | This library is not imported by an endpoint and is not deployed. PAY-001B/C/D must define exact race, merchandise, lookup, refund, admin, nested-object, and custom-field schemas; apply smaller business limits before every write/provider call; store immutable schema/price snapshots; and add endpoint no-side-effect tests. Safe-log primitives do not replace approved observability purpose, retention, access, transport, or provider controls. | | RISK-001, RISK-002, RISK-034 | PAY-003A [#101](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/101) source merged as `87bd1210575882cdd06214bcb98ed38ce00f71c2`, adding durable Stripe Event deduplication, transactional transition/processed marking, paid/amount/currency/Checkout-mode/reference-shape and explicitly configured livemode checks, async outcomes, monotonic refund/terminal guards, quarantine, generic signature failures, and a webhook-secret-only binding. | Firebase was not deployed. Complete CONFIG/PAY-001/PAY-002 contracts, metadata schema allowlist/migration, canonical reducers/reservations/outbox, retry/dead-letter/TTL/alerts, emulator integration, Stripe test-mode rehearsal, protected deploy, and reconcile. | | RISK-006, part of RISK-024/RISK-032 | #100 merged at `a7fc301e85b0aeabe396e771faea21d3fc8e7b2b`. OAuth secrets and server-owned financial/operational collections are denied; current catalog writes are restricted to inert drafts/display fields. | Firebase deployment skipped. Stage/deploy the exact Rules revision under #105, verify synthetic deny behavior, then move remaining catalog mutation to scoped APIs under ADMIN-001. | diff --git a/src/services/firebase/FirebaseResources.test.ts b/src/services/firebase/FirebaseResources.test.ts index 0de7070b..af69e748 100644 --- a/src/services/firebase/FirebaseResources.test.ts +++ b/src/services/firebase/FirebaseResources.test.ts @@ -20,6 +20,10 @@ const mockConnectFunctionsEmulator = jest.fn(); const mockGetAnalytics = jest.fn(() => ({ name: 'test-analytics' })); const mockIsAnalyticsSupported = jest.fn(() => Promise.resolve(false)); const mockInitializeAppCheck = jest.fn(); +const mockReCaptchaEnterpriseProvider = jest.fn((siteKey: string) => ({ + kind: 'enterprise', + siteKey, +})); const mockReCaptchaV3Provider = jest.fn((siteKey: string) => ({ siteKey })); jest.mock('firebase/app', () => ({ initializeApp: mockInitializeApp })); @@ -29,6 +33,7 @@ jest.mock('firebase/analytics', () => ({ })); jest.mock('firebase/app-check', () => ({ initializeAppCheck: mockInitializeAppCheck, + ReCaptchaEnterpriseProvider: mockReCaptchaEnterpriseProvider, ReCaptchaV3Provider: mockReCaptchaV3Provider, })); jest.mock('firebase/auth', () => ({ @@ -93,6 +98,10 @@ describe('FirebaseResources environment isolation', () => { mockGetFunctions.mockReturnValue(mockFunctions); mockGetAnalytics.mockReturnValue({ name: 'test-analytics' }); mockInitializeAppCheck.mockReturnValue({ name: 'test-app-check' }); + mockReCaptchaEnterpriseProvider.mockImplementation((siteKey: string) => ({ + kind: 'enterprise', + siteKey, + })); mockReCaptchaV3Provider.mockImplementation((siteKey: string) => ({ siteKey })); mockConnectAuthEmulator.mockImplementation(() => undefined); mockConnectFirestoreEmulator.mockImplementation(() => undefined); @@ -133,6 +142,7 @@ describe('FirebaseResources environment isolation', () => { 5001, ); expect(mockInitializeAppCheck).not.toHaveBeenCalled(); + expect(mockReCaptchaEnterpriseProvider).not.toHaveBeenCalled(); expect(mockReCaptchaV3Provider).not.toHaveBeenCalled(); expect(mockIsAnalyticsSupported).not.toHaveBeenCalled(); expect(mockGetAnalytics).not.toHaveBeenCalled(); @@ -163,19 +173,36 @@ describe('FirebaseResources environment isolation', () => { expect(mockConnectFirestoreEmulator).toHaveBeenCalled(); expect(mockConnectFunctionsEmulator).toHaveBeenCalled(); expect(mockInitializeAppCheck).not.toHaveBeenCalled(); + expect(mockReCaptchaEnterpriseProvider).not.toHaveBeenCalled(); expect(mockReCaptchaV3Provider).not.toHaveBeenCalled(); expect(mockIsAnalyticsSupported).not.toHaveBeenCalled(); }); - test('production retains its project without starting Analytics', async () => { + test('production uses only the Enterprise App Check provider without Analytics', async () => { + const enterpriseProvider = { + kind: 'enterprise', + siteKey: 'configured-public-site-key', + }; + mockReCaptchaEnterpriseProvider.mockReturnValueOnce(enterpriseProvider); const resources = createResourcesFor('production', 'configured-public-site-key'); expect(resources.app.options.projectId).toBe('mid-peninsula-running-club'); expect(mockConnectAuthEmulator).not.toHaveBeenCalled(); expect(mockConnectFirestoreEmulator).not.toHaveBeenCalled(); expect(mockConnectFunctionsEmulator).not.toHaveBeenCalled(); - expect(mockReCaptchaV3Provider).toHaveBeenCalledWith('configured-public-site-key'); - expect(mockInitializeAppCheck).toHaveBeenCalled(); + expect(mockReCaptchaEnterpriseProvider).toHaveBeenCalledTimes(1); + expect(mockReCaptchaEnterpriseProvider).toHaveBeenCalledWith( + 'configured-public-site-key', + ); + expect(mockReCaptchaV3Provider).not.toHaveBeenCalled(); + expect(mockInitializeAppCheck).toHaveBeenCalledTimes(1); + expect(mockInitializeAppCheck).toHaveBeenCalledWith( + resources.app, + { + provider: enterpriseProvider, + isTokenAutoRefreshEnabled: true, + }, + ); expect(mockIsAnalyticsSupported).not.toHaveBeenCalled(); expect(mockGetAnalytics).not.toHaveBeenCalled(); expect(resources.analytics).toBeNull(); @@ -195,7 +222,10 @@ describe('FirebaseResources environment isolation', () => { expect(resources.auth).toBe(mockAuth); expect(resources.firestore).toBe(mockFirestore); expect(resources.functions).toBe(mockFunctions); + expect(mockReCaptchaEnterpriseProvider).not.toHaveBeenCalled(); + expect(mockReCaptchaV3Provider).not.toHaveBeenCalled(); expect(mockInitializeAppCheck).not.toHaveBeenCalled(); + expect(consoleWarn).toHaveBeenCalledTimes(1); expect(consoleWarn).toHaveBeenCalledWith('[MPRC client] app_check_disabled'); } finally { consoleWarn.mockRestore(); @@ -224,6 +254,7 @@ describe('FirebaseResources environment isolation', () => { expect(consoleWarn).toHaveBeenCalledWith( '[MPRC client] app_check_initialization_failed', ); + expect(consoleWarn).toHaveBeenCalledTimes(1); const serializedConsole = JSON.stringify(consoleWarn.mock.calls); canaries.forEach((canary) => expect(serializedConsole).not.toContain(canary)); } finally { @@ -231,6 +262,30 @@ describe('FirebaseResources environment isolation', () => { } }); + test('Enterprise provider failure continues without logging provider details', () => { + const canary = 'enterprise-provider-private-canary'; + mockReCaptchaEnterpriseProvider.mockImplementationOnce(() => { + throw new Error(canary); + }); + const consoleWarn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + try { + const resources = createResourcesFor('production', 'configured-public-site-key'); + + expect(resources.auth).toBe(mockAuth); + expect(resources.firestore).toBe(mockFirestore); + expect(resources.functions).toBe(mockFunctions); + expect(mockInitializeAppCheck).not.toHaveBeenCalled(); + expect(consoleWarn).toHaveBeenCalledTimes(1); + expect(consoleWarn).toHaveBeenCalledWith( + '[MPRC client] app_check_initialization_failed', + ); + expect(JSON.stringify(consoleWarn.mock.calls)).not.toContain(canary); + } finally { + consoleWarn.mockRestore(); + } + }); + test.each([ '/account/strava/callback?code=example&state=example-state', '/register/success?registration=r1&token=example-capability', @@ -244,6 +299,7 @@ describe('FirebaseResources environment isolation', () => { ])('production keeps App Check and Analytics off a capability callback: %s', (pathName) => { createResourcesFor('production', 'configured-public-site-key', pathName); + expect(mockReCaptchaEnterpriseProvider).not.toHaveBeenCalled(); expect(mockReCaptchaV3Provider).not.toHaveBeenCalled(); expect(mockInitializeAppCheck).not.toHaveBeenCalled(); expect(mockIsAnalyticsSupported).not.toHaveBeenCalled(); @@ -267,6 +323,13 @@ describe('FirebaseResources environment isolation', () => { expect(resources.analytics).toBeNull(); }); + test('production source contains no legacy v3 App Check provider', () => { + const source = fs.readFileSync(path.join(__dirname, 'FirebaseResources.ts'), 'utf8'); + + expect(source).toContain('ReCaptchaEnterpriseProvider'); + expect(source).not.toContain('ReCaptchaV3Provider'); + }); + test('rejects malformed direct Function names before building a URL', () => { const resources = createResourcesFor('development'); diff --git a/src/services/firebase/FirebaseResources.ts b/src/services/firebase/FirebaseResources.ts index b493961e..d6f0d19b 100644 --- a/src/services/firebase/FirebaseResources.ts +++ b/src/services/firebase/FirebaseResources.ts @@ -1,6 +1,6 @@ import { FirebaseApp, initializeApp } from 'firebase/app'; import { - AppCheck, initializeAppCheck, ReCaptchaV3Provider, + AppCheck, initializeAppCheck, ReCaptchaEnterpriseProvider, } from 'firebase/app-check'; import { Auth, connectAuthEmulator, getAuth } from 'firebase/auth'; import { @@ -77,7 +77,7 @@ class FirebaseResources { private initAppCheck(): void { // App Check has no local Emulator Suite target. Do not initialize a // provider or exchange a debug token from development or tests. - // Its reCAPTCHA provider is also an outside script, so do not start it + // Its reCAPTCHA Enterprise provider is also an outside script, so do not start it // while an initial OAuth/checkout capability remains in the page URL. if ( isLocalRuntime @@ -91,7 +91,7 @@ class FirebaseResources { } try { this._appCheck = initializeAppCheck(this.app, { - provider: new ReCaptchaV3Provider(siteKey), + provider: new ReCaptchaEnterpriseProvider(siteKey), isTokenAutoRefreshEnabled: true, }); } catch {