From fe300f11d8f550e098cd9fc4860a23bdd46c07a6 Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 00:28:09 -0700 Subject: [PATCH 1/9] SAFETY-001: isolate callbacks and local Firebase --- IMPLEMENTATION_PLAN.md | 6 +- SYSTEM_DESIGN.md | 45 +++- functions/package.json | 6 +- package.json | 2 + public/404.html | 7 +- public/index.html | 14 +- public/spa-navigation.js | 95 ++++++++ .../admin/events/AdminEventRegistrations.tsx | 4 +- .../firebase/FirebaseResources.test.ts | 219 ++++++++++++++++++ src/services/firebase/FirebaseResources.ts | 75 ++++-- src/services/monitoring/sentry.test.ts | 70 ++++++ src/services/monitoring/sentry.ts | 28 ++- tests/spa-navigation.test.js | 149 ++++++++++++ 13 files changed, 672 insertions(+), 48 deletions(-) create mode 100644 public/spa-navigation.js create mode 100644 src/services/firebase/FirebaseResources.test.ts create mode 100644 src/services/monitoring/sentry.test.ts create mode 100644 tests/spa-navigation.test.js diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md index b1f38c44..96a29007 100644 --- a/IMPLEMENTATION_PLAN.md +++ b/IMPLEMENTATION_PLAN.md @@ -92,7 +92,7 @@ Exit gate: - Unused vulnerable dependency chains are removed and direct safe patches applied. - Live checkout remains disabled and placeholder policies are visible as a launch blocker. -**Assessment snapshot, not current `main`:** the combined working tree contains a locally verified SAFETY-001 candidate, but issue [#99](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/99) is queued behind #104. Until #99 is isolated, reviewed, merged, and reverified, callback/emulator behavior is **NOT AVAILABLE YET**. +**Current source boundary:** issue [#99](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/99) provides the isolated callback and local Firebase safety slice: a same-origin path/query/fragment bridge, a fully synthetic local Firebase namespace, loopback Auth/Firestore/Functions routing, fail-closed setup, and local monitoring shutdown. Its exact issue/PR record remains the status source. This does not create staging, isolate provider calls made by Functions, prove a production callback, or repair a member profile. ### Phase 1 — Establish trust boundaries @@ -206,9 +206,9 @@ Use structured correlation across checkout request, business record, Stripe Sess The first tranche deliberately combines one urgent safety repair with the most complex payment foundation: -1. **SAFETY-001 — callback and emulator isolation:** preserve path/query/hash through GitHub Pages and connect local Functions to the emulator. This prevents paid/OAuth callbacks from breaking and local code from reaching production unexpectedly. +1. **SAFETY-001 — callback and emulator isolation:** #99 supplies the source/test boundary for same-origin callback preservation and loopback Auth/Firestore/Functions development. Provider calls, optimized previews, staging, and live callbacks remain separately gated. 2. **PAY-003 — webhook event inbox and safe reducer:** add event replay protection, async-aware payment confirmation, expected total/currency validation, direct metadata lookup, terminal-state guards, and lifecycle tests. -3. **SEC-001 — Firestore trust boundaries:** remove recursive browser-admin authority, protect secrets/financial state, retain only allowlisted catalog administration, and reverse unsafe rule tests. This is implemented locally and awaits review/deployment evidence. +3. **SEC-001 — Firestore trust boundaries:** #100 merged through [PR #123](https://github.com/Run-MPRC/Run-MPRC.github.io/pull/123) at `a7fc301e85b0aeabe396e771faea21d3fc8e7b2b`. Source and 295 Rules tests remove recursive browser-admin authority. Firebase deployment explicitly skipped, so deployed/live Rules remain unproven under #105. 4. **PROMO-001 — fail-safe monetary policy:** disable promotion entry and quarantine unexplained discount/tax/shipping adjustments. This safety default is implemented locally; creator tests/provider inventory remain. 5. **Identity/config next:** AUTH-001A [#98](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/98) merged at `ce22c110`, proving source/tests for unverified-target rejection at the existing role-grant endpoints. Its Firebase deployment skipped, so backend-live status is unproven and the remaining AUTH-001 parent work stays open. Continue CONFIG-001 and the uncompleted AUTH-001 guards/Rules/refresh/audit scope without duplicating #98. 6. **Then:** PAY-001/PAY-002 so checkout creation, refunds, and every later handler share strict schemas and idempotent state. diff --git a/SYSTEM_DESIGN.md b/SYSTEM_DESIGN.md index a699267e..18997b18 100644 --- a/SYSTEM_DESIGN.md +++ b/SYSTEM_DESIGN.md @@ -87,11 +87,30 @@ flowchart LR | Payments | Stripe Checkout Sessions, Payment Links, refunds, signed webhook | `functions/createCheckoutSession.js`, `createMerchCheckout.js`, `stripeWebhook.js` | Not ready for live payments until P0 issues are complete. | | Hosting | Netlify currently answers `runmprc.com`; GitHub Actions also publishes a separate Pages copy | `netlify.toml`, `.github/workflows/deploy.yml`, `public/404.html` | Split and drifting as verified 2026-07-12. A Pages success does not prove the live Netlify site changed. Hosting authority, preview, DNS, headers, and rollback remain open CI/WEB work. | | Email | Firestore `mail` outbox designed for the Firebase Trigger Email extension | `functions/sendConfirmationEmail.js` | Extension/provider deployment is unverified; outbox creation is not transactionally idempotent and HTML needs escaping. | -| Observability | Optional Sentry and Firebase Analytics | `src/services/monitoring`, `src/services/analytics` | Local initialization is disabled in the working tree; hosted policy/config is unverified and no payment-specific alerts, metrics, or reconciliation dashboard exists. | +| Observability | Optional Sentry and Firebase Analytics | `src/services/monitoring`, `src/services/analytics` | Local/test initialization is disabled by #99 source. Hosted redaction, replay, consent, retention, and provider configuration remain unverified under #111. | | Third-party fitness | Strava OAuth tokens and statistics | `functions/strava.js`, `src/services/strava` | Functional prototype. Working-tree Rules deny browser token access, but transactional refresh, scopes/revocation, IAM/encryption decision, and audit remain OAUTH-001. | The repository workflow presents GitHub Pages plus Firebase as a release path, but public DNS and response headers show that the live custom domain is served by Netlify. The two copies currently contain different bundles. The Firebase job can also finish green while explicitly skipping deployment when `FIREBASE_SERVICE_ACCOUNT` is absent. Treat Pages publication, Netlify production, and Firebase deployment as separate evidence until one hosting/release authority is chosen. The App Engine synchronization script is another surface that must be documented as active or retired. +### GitHub Pages callback handoff + +```mermaid +flowchart LR + Provider["Stripe or Strava returns to a deep link"] + Missing["GitHub Pages 404"] + Store["Store same-origin path, query, and fragment\nin this browser tab"] + Root["Redirect to the site root"] + Restore["Remove temporary state, validate origin,\nand restore the route once"] + Router["React Router opens the callback page"] + Reject["Discard malformed or outside-site target"] + + Provider --> Missing --> Store --> Root --> Restore + Restore -->|safe| Router + Restore -->|unsafe| Reject +``` + +Text alternative: the 404 page temporarily carries the complete return route to the root page; the root page deletes that temporary value before accepting only a same-origin route. The bridge does not prove payment, OAuth state, or identity. Server/provider verification still decides the result. + ## 4. Target deployment topology The target keeps React, Firebase, and Stripe, but places stronger boundaries around them. Migrating the frontend from GitHub Pages to Firebase Hosting is recommended before live commerce because it supports controlled SPA rewrites, preview channels, and security headers. That hosting migration is not required to design or test the backend, and must be executed as its own issue. @@ -296,12 +315,32 @@ Returning a `2xx` response to Stripe means the event has been durably accepted o | Environment | Firebase | Stripe | Domain | Data policy | | --- | --- | --- | --- | --- | -| Local | Firebase Emulator Suite | Stripe CLI and test objects | `localhost` | Synthetic data only; no production network fallback | +| Local source runtime | Firebase Emulator Suite under `demo-mprc-local` | Not safe by Firebase emulation alone | `localhost:3000` | Synthetic data only; browser Firebase traffic is loopback-only | | CI | Ephemeral emulators and mocks | Stripe fixtures/signature tests; optional isolated test account | None | Synthetic data, no production secrets | | Staging | Dedicated non-production Firebase project | Stripe sandbox/test keys and its own webhook endpoint | `dev.runmprc.com` | Synthetic or consented test records only | | Production | Dedicated production Firebase project | Live restricted keys and production webhook secret | `runmprc.com` | Real data under documented retention and access policies | -`SITE_ORIGIN`, Firebase project IDs, App Check keys, Stripe keys, webhook endpoints, Sentry environments, and email providers must be environment-scoped. Live and test webhook secrets are never interchangeable. The current repository points to one Firebase project and needs an explicit staging project before end-to-end rollout. +```mermaid +flowchart LR + Browser["Local React app\nlocalhost:3000"] + Auth["Auth emulator\n127.0.0.1:9099"] + Firestore["Firestore emulator\n127.0.0.1:8080"] + Functions["Functions emulator\n127.0.0.1:5001"] + Providers["Stripe, Strava, email,\nand other outside providers"] + + Browser --> Auth + Browser --> Firestore + Browser --> Functions + Functions -. "NOT isolated by Firebase" .-> Providers +``` + +Text alternative: the local browser uses only the three loopback Firebase emulators. A Function can still call an outside provider, so Firebase emulator readiness alone does not authorize checkout, refund, email, or Strava testing. + +#99 source selects a fully synthetic Firebase configuration for development/test, connects Auth, Firestore, and Functions to the loopback ports above, stops startup when connection setup fails, and leaves App Check, Analytics, and Sentry off locally. The Firebase CLI must still report all three emulators ready before the app is opened. Mocked endpoint tests alone do not prove listening processes. + +Optimized builds—including current Netlify previews and a locally served `build/` directory—use `NODE_ENV=production` and still target production Firebase. They are restricted to public, read-only visual review until #105/CONFIG establishes a separate staging configuration. Do not sign in, open private/admin pages, or test Firebase/provider actions in those previews. + +`SITE_ORIGIN`, Firebase project IDs, App Check keys, Stripe keys, webhook endpoints, Sentry environments, and email providers must be environment-scoped. Live and test webhook secrets are never interchangeable. The repository still needs an explicit staging project before end-to-end rollout. ## 11. Security, privacy, and compliance posture diff --git a/functions/package.json b/functions/package.json index 892fcb59..5784ad38 100644 --- a/functions/package.json +++ b/functions/package.json @@ -3,15 +3,15 @@ "description": "Cloud Functions for Firebase - MPRC Backend", "scripts": { "build": "echo 'no build step'", - "serve": "firebase emulators:start --only functions", - "shell": "firebase functions:shell", + "serve": "firebase emulators:start --project demo-mprc-local --only functions", + "shell": "firebase functions:shell --project demo-mprc-local", "start": "npm run shell", "deploy": "firebase deploy --only functions", "deploy:prod": "firebase deploy --only functions --project production", "logs": "firebase functions:log", "lint": "eslint .", "lint:fix": "eslint . --fix", - "test": "firebase emulators:exec --only firestore,auth 'npm run test:run'", + "test": "firebase emulators:exec --project demo-functions-test --only firestore,auth 'npm run test:run'", "test:run": "jest" }, "engines": { diff --git a/package.json b/package.json index 0d142ab0..0c243318 100644 --- a/package.json +++ b/package.json @@ -30,8 +30,10 @@ "build": "react-scripts build", "predeploy": "npm run build", "deploy": "gh-pages -d build", + "emulators": "firebase emulators:start --project demo-mprc-local --only auth,firestore,functions", "lint:fix": "eslint --fix --ext .js,.jsx .", "test": "react-scripts test", + "test:spa-navigation": "node --test tests/spa-navigation.test.js", "test:rules": "firebase emulators:exec --only firestore --project demo-rules-test \"npx --no-install jest --config tests/firestore-rules/jest.config.js\"", "test:rules:watch": "firebase emulators:exec --only firestore --project demo-rules-test \"npx --no-install jest --config tests/firestore-rules/jest.config.js --watch\"", "eject": "react-scripts eject", diff --git a/public/404.html b/public/404.html index 560606f1..45218775 100644 --- a/public/404.html +++ b/public/404.html @@ -3,12 +3,7 @@ Redirecting... - + diff --git a/public/index.html b/public/index.html index b282136e..5fb169e4 100644 --- a/public/index.html +++ b/public/index.html @@ -160,16 +160,10 @@ - +
diff --git a/public/spa-navigation.js b/public/spa-navigation.js new file mode 100644 index 00000000..3e6ee634 --- /dev/null +++ b/public/spa-navigation.js @@ -0,0 +1,95 @@ +((root, factory) => { + const api = factory(); + + if (typeof module === 'object' && module.exports) { + module.exports = api; + } + + if (!root) return; + + const script = root.document && root.document.currentScript; + const action = script && script.getAttribute('data-mprc-spa-action'); + + if (action === 'capture') { + api.captureRedirect(root); + } else if (action === 'restore') { + api.restoreRedirect(root); + } +})(typeof window === 'undefined' ? null : window, () => { + const STORAGE_KEY = 'mprc:spa-redirect'; + + function buildRedirectTarget(location) { + const pathname = typeof location.pathname === 'string' + && location.pathname.startsWith('/') + ? location.pathname + : '/'; + const search = typeof location.search === 'string' + && (location.search === '' || location.search.startsWith('?')) + ? location.search + : ''; + const hash = typeof location.hash === 'string' + && (location.hash === '' || location.hash.startsWith('#')) + ? location.hash + : ''; + return pathname + search + hash; + } + + function parseSameOriginTarget(storedTarget, origin) { + if (typeof storedTarget !== 'string' || !storedTarget.startsWith('/')) { + return null; + } + + try { + const normalizedOrigin = new URL(origin).origin; + const target = new URL(storedTarget, normalizedOrigin); + if (target.origin !== normalizedOrigin) return null; + return target.pathname + target.search + target.hash; + } catch (_error) { + return null; + } + } + + function captureRedirect(browserWindow) { + const target = buildRedirectTarget(browserWindow.location); + try { + browserWindow.sessionStorage.setItem(STORAGE_KEY, target); + } catch (_error) { + browserWindow.location.replace('/'); + return false; + } + browserWindow.location.replace('/'); + return true; + } + + function restoreRedirect(browserWindow) { + let storedTarget; + try { + storedTarget = browserWindow.sessionStorage.getItem(STORAGE_KEY); + browserWindow.sessionStorage.removeItem(STORAGE_KEY); + } catch (_error) { + return false; + } + if (!storedTarget) return false; + + const target = parseSameOriginTarget( + storedTarget, + browserWindow.location.origin, + ); + if (!target) return false; + + try { + browserWindow.history.replaceState(null, '', target); + return true; + } catch (_error) { + return false; + } + } + + return { + STORAGE_KEY, + buildRedirectTarget, + parseSameOriginTarget, + captureRedirect, + restoreRedirect, + }; +}); diff --git a/src/pages/admin/events/AdminEventRegistrations.tsx b/src/pages/admin/events/AdminEventRegistrations.tsx index 50e03b29..111d9b8e 100644 --- a/src/pages/admin/events/AdminEventRegistrations.tsx +++ b/src/pages/admin/events/AdminEventRegistrations.tsx @@ -188,8 +188,8 @@ function Inner() { try { const token = await services.firebaseResources.auth.currentUser?.getIdToken(); if (!token) throw new Error('Not signed in'); - const projectId = services.firebaseResources.app.options.projectId; - const url = `https://us-central1-${projectId}.cloudfunctions.net/exportRegistrationsCsv?eventId=${encodeURIComponent(slug)}`; + const endpoint = services.firebaseResources.getHttpFunctionUrl('exportRegistrationsCsv'); + const url = `${endpoint}?eventId=${encodeURIComponent(slug)}`; const resp = await fetch(url, { headers: { Authorization: `Bearer ${token}` }, }); diff --git a/src/services/firebase/FirebaseResources.test.ts b/src/services/firebase/FirebaseResources.test.ts new file mode 100644 index 00000000..113dc33d --- /dev/null +++ b/src/services/firebase/FirebaseResources.test.ts @@ -0,0 +1,219 @@ +export {}; + +const fs = require('node:fs'); +const path = require('node:path'); + +const mockAuth = { name: 'test-auth' }; +const mockFirestore = { name: 'test-firestore' }; +const mockFunctions = { name: 'test-functions' }; + +const mockInitializeApp = jest.fn((options: Record) => ({ + name: 'test-app', + options, +})); +const mockGetAuth = jest.fn(() => mockAuth); +const mockGetFirestore = jest.fn(() => mockFirestore); +const mockGetFunctions = jest.fn(() => mockFunctions); +const mockConnectAuthEmulator = jest.fn(); +const mockConnectFirestoreEmulator = jest.fn(); +const mockConnectFunctionsEmulator = jest.fn(); +const mockGetAnalytics = jest.fn(() => ({ name: 'test-analytics' })); +const mockIsAnalyticsSupported = jest.fn(() => Promise.resolve(false)); +const mockInitializeAppCheck = jest.fn(); +const mockReCaptchaV3Provider = jest.fn((siteKey: string) => ({ siteKey })); + +jest.mock('firebase/app', () => ({ initializeApp: mockInitializeApp })); +jest.mock('firebase/analytics', () => ({ + getAnalytics: mockGetAnalytics, + isSupported: mockIsAnalyticsSupported, +})); +jest.mock('firebase/app-check', () => ({ + initializeAppCheck: mockInitializeAppCheck, + ReCaptchaV3Provider: mockReCaptchaV3Provider, +})); +jest.mock('firebase/auth', () => ({ + connectAuthEmulator: mockConnectAuthEmulator, + getAuth: mockGetAuth, +})); +jest.mock('firebase/firestore', () => ({ + connectFirestoreEmulator: mockConnectFirestoreEmulator, + getFirestore: mockGetFirestore, +})); +jest.mock('firebase/functions', () => ({ + connectFunctionsEmulator: mockConnectFunctionsEmulator, + getFunctions: mockGetFunctions, +})); + +function setNodeEnv(value: string | undefined) { + Object.defineProperty(process.env, 'NODE_ENV', { + configurable: true, + value, + writable: true, + }); +} + +function createResourcesFor(nodeEnv: string, siteKey?: string) { + const originalNodeEnv = process.env.NODE_ENV; + const originalSiteKey = process.env.REACT_APP_RECAPTCHA_SITE_KEY; + setNodeEnv(nodeEnv); + if (siteKey === undefined) delete process.env.REACT_APP_RECAPTCHA_SITE_KEY; + else process.env.REACT_APP_RECAPTCHA_SITE_KEY = siteKey; + + try { + let resources: import('./FirebaseResources').default; + jest.isolateModules(() => { + // eslint-disable-next-line global-require + const FirebaseResources = require('./FirebaseResources').default; + resources = FirebaseResources.getInstance(); + }); + return resources!; + } finally { + setNodeEnv(originalNodeEnv); + if (originalSiteKey === undefined) delete process.env.REACT_APP_RECAPTCHA_SITE_KEY; + else process.env.REACT_APP_RECAPTCHA_SITE_KEY = originalSiteKey; + } +} + +describe('FirebaseResources environment isolation', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockInitializeApp.mockImplementation((options: Record) => ({ + name: 'test-app', + options, + })); + mockGetAuth.mockReturnValue(mockAuth); + mockGetFirestore.mockReturnValue(mockFirestore); + mockGetFunctions.mockReturnValue(mockFunctions); + mockGetAnalytics.mockReturnValue({ name: 'test-analytics' }); + mockInitializeAppCheck.mockReturnValue({ name: 'test-app-check' }); + mockReCaptchaV3Provider.mockImplementation((siteKey: string) => ({ siteKey })); + mockConnectAuthEmulator.mockImplementation(() => undefined); + mockConnectFirestoreEmulator.mockImplementation(() => undefined); + mockConnectFunctionsEmulator.mockImplementation(() => undefined); + mockIsAnalyticsSupported.mockImplementation(() => Promise.resolve(false)); + }); + + test('development uses only the demo namespace and all three emulators', () => { + const resources = createResourcesFor('development', 'configured-public-site-key'); + + expect(resources.app.options).toEqual(expect.objectContaining({ + apiKey: 'demo-api-key', + authDomain: 'demo-mprc-local.firebaseapp.com', + projectId: 'demo-mprc-local', + storageBucket: 'demo-mprc-local.appspot.com', + })); + expect(resources.app.options).not.toEqual(expect.objectContaining({ + projectId: 'mid-peninsula-running-club', + })); + expect(JSON.stringify(resources.app.options)).not.toContain( + 'mid-peninsula-running-club', + ); + expect(JSON.stringify(resources.app.options)).not.toContain('253289716314'); + expect(resources.functions).toBe(mockFunctions); + expect(mockConnectAuthEmulator).toHaveBeenCalledWith( + mockAuth, + 'http://127.0.0.1:9099', + { disableWarnings: true }, + ); + expect(mockConnectFirestoreEmulator).toHaveBeenCalledWith( + mockFirestore, + '127.0.0.1', + 8080, + ); + expect(mockConnectFunctionsEmulator).toHaveBeenCalledWith( + mockFunctions, + '127.0.0.1', + 5001, + ); + expect(mockInitializeAppCheck).not.toHaveBeenCalled(); + expect(mockReCaptchaV3Provider).not.toHaveBeenCalled(); + expect(mockIsAnalyticsSupported).not.toHaveBeenCalled(); + expect(mockGetAnalytics).not.toHaveBeenCalled(); + expect(resources.getHttpFunctionUrl('exportRegistrationsCsv')).toBe( + 'http://127.0.0.1:5001/demo-mprc-local/us-central1/exportRegistrationsCsv', + ); + }); + + test.each([ + ['Auth', mockConnectAuthEmulator], + ['Firestore', mockConnectFirestoreEmulator], + ['Functions', mockConnectFunctionsEmulator], + ])('development startup fails closed when %s cannot be configured', (_name, connector) => { + connector.mockImplementationOnce(() => { + throw new Error('connection setup failed'); + }); + + expect(() => createResourcesFor('development')).toThrow( + 'Local Firebase emulator isolation failed; stop development startup.', + ); + }); + + test('test runtime also uses the demo project and local emulators', () => { + const resources = createResourcesFor('test', 'configured-public-site-key'); + + expect(resources.app.options.projectId).toBe('demo-mprc-local'); + expect(mockConnectAuthEmulator).toHaveBeenCalled(); + expect(mockConnectFirestoreEmulator).toHaveBeenCalled(); + expect(mockConnectFunctionsEmulator).toHaveBeenCalled(); + expect(mockInitializeAppCheck).not.toHaveBeenCalled(); + expect(mockReCaptchaV3Provider).not.toHaveBeenCalled(); + expect(mockIsAnalyticsSupported).not.toHaveBeenCalled(); + }); + + test('production retains its project and never connects an emulator', async () => { + 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(mockIsAnalyticsSupported).toHaveBeenCalled(); + await Promise.resolve(); + expect(resources.getHttpFunctionUrl('exportRegistrationsCsv')).toBe( + 'https://us-central1-mid-peninsula-running-club.cloudfunctions.net/' + + 'exportRegistrationsCsv', + ); + }); + + test('rejects malformed direct Function names before building a URL', () => { + const resources = createResourcesFor('development'); + + expect(() => resources.getHttpFunctionUrl('../other')).toThrow( + 'Invalid Firebase Function name.', + ); + expect(() => resources.getHttpFunctionUrl('https://example.test')).toThrow( + 'Invalid Firebase Function name.', + ); + }); + + test('the direct CSV export uses the environment-aware Function URL helper', () => { + const source = fs.readFileSync( + path.join(__dirname, '../../pages/admin/events/AdminEventRegistrations.tsx'), + 'utf8', + ); + + expect(source).toMatch( + /firebaseResources\.getHttpFunctionUrl\('exportRegistrationsCsv'\)/, + ); + expect(source).not.toMatch(/cloudfunctions\.net/); + }); + + test('the root and Functions emulator scripts select demo-only projects', () => { + const rootPackage = JSON.parse(fs.readFileSync( + path.join(__dirname, '../../../package.json'), + 'utf8', + )); + const functionsPackage = JSON.parse(fs.readFileSync( + path.join(__dirname, '../../../functions/package.json'), + 'utf8', + )); + + expect(rootPackage.scripts.emulators).toContain('--project demo-mprc-local'); + expect(rootPackage.scripts.emulators).toContain('--only auth,firestore,functions'); + expect(functionsPackage.scripts.serve).toContain('--project demo-mprc-local'); + expect(functionsPackage.scripts.shell).toContain('--project demo-mprc-local'); + expect(functionsPackage.scripts.test).toContain('--project demo-functions-test'); + }); +}); diff --git a/src/services/firebase/FirebaseResources.ts b/src/services/firebase/FirebaseResources.ts index eb609763..fb222e91 100644 --- a/src/services/firebase/FirebaseResources.ts +++ b/src/services/firebase/FirebaseResources.ts @@ -9,8 +9,26 @@ import { Firestore, getFirestore, } from 'firebase/firestore'; +import { + connectFunctionsEmulator, + Functions, + getFunctions, +} from 'firebase/functions'; + +const isLocalRuntime = process.env.NODE_ENV !== 'production'; +const LOCAL_FIREBASE_PROJECT_ID = 'demo-mprc-local'; +const FUNCTION_NAME_PATTERN = /^[A-Za-z][A-Za-z0-9_-]*$/; + +const LOCAL_FIREBASE_CONFIG = { + apiKey: 'demo-api-key', + authDomain: `${LOCAL_FIREBASE_PROJECT_ID}.firebaseapp.com`, + projectId: LOCAL_FIREBASE_PROJECT_ID, + storageBucket: `${LOCAL_FIREBASE_PROJECT_ID}.appspot.com`, + messagingSenderId: '000000000000', + appId: '1:000000000000:web:demo', +} as const; -const FIREBASE_CONFIG = { +const PRODUCTION_FIREBASE_CONFIG = { apiKey: 'AIzaSyD2u17HMhDPZ0Tn9D3H71fep1vZgT-njnw', authDomain: 'mid-peninsula-running-club.firebaseapp.com', projectId: 'mid-peninsula-running-club', @@ -20,7 +38,9 @@ const FIREBASE_CONFIG = { measurementId: 'G-ECN7TT0BGF', } as const; -const isDevelopment = process.env.NODE_ENV === 'development'; +const FIREBASE_CONFIG = isLocalRuntime + ? LOCAL_FIREBASE_CONFIG + : PRODUCTION_FIREBASE_CONFIG; class FirebaseResources { readonly app: FirebaseApp; @@ -29,6 +49,8 @@ class FirebaseResources { readonly firestore: Firestore; + readonly functions: Functions; + private _analytics: Analytics | null = null; private _appCheck: AppCheck | null = null; @@ -42,25 +64,24 @@ class FirebaseResources { this.initAppCheck(); this.auth = getAuth(this.app); this.firestore = getFirestore(this.app); + this.functions = getFunctions(this.app); this.connectEmulators(); this.initAnalytics(); } 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. + if (isLocalRuntime) return; + const siteKey = process.env.REACT_APP_RECAPTCHA_SITE_KEY; if (!siteKey) { - if (!isDevelopment) { - console.warn( - 'App Check disabled: set REACT_APP_RECAPTCHA_SITE_KEY to enable', - ); - } + console.warn( + 'App Check disabled: set REACT_APP_RECAPTCHA_SITE_KEY to enable', + ); return; } - if (isDevelopment) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (globalThis as any).FIREBASE_APPCHECK_DEBUG_TOKEN = true; - } try { this._appCheck = initializeAppCheck(this.app, { provider: new ReCaptchaV3Provider(siteKey), @@ -79,18 +100,26 @@ class FirebaseResources { } private connectEmulators(): void { - if (isDevelopment && !FirebaseResources._emulatorsConnected) { + if (isLocalRuntime && !FirebaseResources._emulatorsConnected) { try { - connectAuthEmulator(this.auth, 'http://localhost:9099', { disableWarnings: true }); + connectAuthEmulator(this.auth, 'http://127.0.0.1:9099', { disableWarnings: true }); connectFirestoreEmulator(this.firestore, '127.0.0.1', 8080); + connectFunctionsEmulator(this.functions, '127.0.0.1', 5001); FirebaseResources._emulatorsConnected = true; - } catch (error) { - console.warn('Failed to connect to Firebase emulators:', error); + } catch { + // Continuing after a partial connection could send a later SDK call + // to a live service. Stop local startup instead. + throw new Error( + 'Local Firebase emulator isolation failed; stop development startup.', + ); } } } private async initAnalytics(): Promise { + // Analytics has no Emulator Suite target. Never initialize it from local + // development or tests, even when a production measurement ID is present. + if (isLocalRuntime) return; try { const supported = await isSupported(); if (supported) { @@ -104,6 +133,22 @@ class FirebaseResources { get analytics(): Analytics | null { return this._analytics; } + + getHttpFunctionUrl(functionName: string): string { + if (!FUNCTION_NAME_PATTERN.test(functionName)) { + throw new Error('Invalid Firebase Function name.'); + } + + const { projectId } = this.app.options; + if (!projectId) { + throw new Error('Firebase project ID is unavailable.'); + } + + if (isLocalRuntime) { + return `http://127.0.0.1:5001/${projectId}/us-central1/${functionName}`; + } + return `https://us-central1-${projectId}.cloudfunctions.net/${functionName}`; + } } export default FirebaseResources; diff --git a/src/services/monitoring/sentry.test.ts b/src/services/monitoring/sentry.test.ts new file mode 100644 index 00000000..936381e8 --- /dev/null +++ b/src/services/monitoring/sentry.test.ts @@ -0,0 +1,70 @@ +export {}; + +const mockInit = jest.fn(); +const mockCaptureException = jest.fn(); +const mockSetUser = jest.fn(); + +jest.mock('@sentry/react', () => ({ + init: mockInit, + captureException: mockCaptureException, + setUser: mockSetUser, +})); + +function setNodeEnv(value: string | undefined) { + Object.defineProperty(process.env, 'NODE_ENV', { + configurable: true, + value, + writable: true, + }); +} + +function initializeFor(nodeEnv: string, locationPath = '/') { + const originalNodeEnv = process.env.NODE_ENV; + const originalDsn = process.env.REACT_APP_SENTRY_DSN; + const originalLocation = `${window.location.pathname}${window.location.search}${window.location.hash}`; + setNodeEnv(nodeEnv); + process.env.REACT_APP_SENTRY_DSN = 'https://public@example.ingest.sentry.io/1'; + window.history.replaceState(null, '', locationPath); + + try { + jest.isolateModules(() => { + // eslint-disable-next-line global-require + require('./sentry').initSentry(); + }); + } finally { + window.history.replaceState(null, '', originalLocation); + setNodeEnv(originalNodeEnv); + if (originalDsn === undefined) delete process.env.REACT_APP_SENTRY_DSN; + else process.env.REACT_APP_SENTRY_DSN = originalDsn; + } +} + +describe('Sentry environment and callback isolation', () => { + beforeEach(() => jest.clearAllMocks()); + + test.each(['development', 'test'])( + 'does not initialize an external client during %s', + (nodeEnv) => { + initializeFor(nodeEnv); + expect(mockInit).not.toHaveBeenCalled(); + }, + ); + + test('allows an explicitly configured ordinary production page to initialize', () => { + initializeFor('production', '/events'); + + expect(mockInit).toHaveBeenCalledWith(expect.objectContaining({ + dsn: 'https://public@example.ingest.sentry.io/1', + })); + }); + + test.each([ + '/account/strava/callback?code=example&state=example-state', + '/register/success?registration=r1&token=example-capability', + '/shop/purchase/success?order=o1&token=example-capability', + '/register/success#example-capability', + ])('does not initialize monitoring on a capability callback: %s', (locationPath) => { + initializeFor('production', locationPath); + expect(mockInit).not.toHaveBeenCalled(); + }); +}); diff --git a/src/services/monitoring/sentry.ts b/src/services/monitoring/sentry.ts index 5982623c..2f0c0690 100644 --- a/src/services/monitoring/sentry.ts +++ b/src/services/monitoring/sentry.ts @@ -9,10 +9,26 @@ import * as Sentry from '@sentry/react'; * 3. Set REACT_APP_SENTRY_DSN in your build environment */ -let _initialized = false; +let initialized = false; +const CAPABILITY_CALLBACK_PATHS = new Set([ + '/account/strava/callback', + '/register/success', + '/shop/purchase/success', +]); + +function hasCapabilityCallbackState(): boolean { + if (typeof window === 'undefined') return false; + const { pathname, search, hash } = window.location; + return CAPABILITY_CALLBACK_PATHS.has(pathname) + && (search.length > 0 || hash.length > 0); +} export function initSentry(): void { - if (_initialized) return; + if (initialized) return; + // Local/test sessions must not reach an outside monitoring service. In + // production, do not initialize on callback URLs carrying OAuth or checkout + // capabilities. #111 owns the broader hosted redaction/replay policy. + if (process.env.NODE_ENV !== 'production' || hasCapabilityCallbackState()) return; const dsn = process.env.REACT_APP_SENTRY_DSN; if (!dsn) return; const environment = process.env.REACT_APP_SENTRY_ENV @@ -25,21 +41,21 @@ export function initSentry(): void { replaysSessionSampleRate: 0, replaysOnErrorSampleRate: 1.0, }); - _initialized = true; + initialized = true; } export function captureException(err: unknown): void { - if (_initialized) { + if (initialized) { Sentry.captureException(err); } } export function setUserContext(user: { uid: string; email?: string | null }): void { - if (!_initialized) return; + if (!initialized) return; Sentry.setUser({ id: user.uid, email: user.email || undefined }); } export function clearUserContext(): void { - if (!_initialized) return; + if (!initialized) return; Sentry.setUser(null); } diff --git a/tests/spa-navigation.test.js b/tests/spa-navigation.test.js new file mode 100644 index 00000000..f7f2060e --- /dev/null +++ b/tests/spa-navigation.test.js @@ -0,0 +1,149 @@ +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const test = require('node:test'); + +const { + STORAGE_KEY, + buildRedirectTarget, + captureRedirect, + parseSameOriginTarget, + restoreRedirect, +} = require('../public/spa-navigation'); + +function storageWith(initial = {}) { + const values = new Map(Object.entries(initial)); + return { + getItem: (key) => values.get(key) ?? null, + setItem: (key, value) => values.set(key, String(value)), + removeItem: (key) => values.delete(key), + has: (key) => values.has(key), + }; +} + +test('captures pathname, query, and hash before redirecting to the root', () => { + const storage = storageWith(); + const replacements = []; + const browserWindow = { + location: { + pathname: '/register/success', + search: '?session_id=cs_test_example®istration=r1', + hash: '#receipt', + replace: (target) => replacements.push(target), + }, + sessionStorage: storage, + }; + + assert.equal(captureRedirect(browserWindow), true); + assert.equal( + storage.getItem(STORAGE_KEY), + '/register/success?session_id=cs_test_example®istration=r1#receipt', + ); + assert.deepEqual(replacements, ['/']); +}); + +test('drops malformed location components while building the stored route', () => { + assert.equal(buildRedirectTarget({ + pathname: 'https://attacker.example/path', + search: 'not-a-query', + hash: 'not-a-hash', + }), '/'); +}); + +test('restores the exact same-origin route and clears temporary state', () => { + const target = '/account/strava/callback?code=example&state=example-state#connected'; + const storage = storageWith({ [STORAGE_KEY]: target }); + const historyCalls = []; + const browserWindow = { + location: { origin: 'https://runmprc.com' }, + sessionStorage: storage, + history: { + replaceState: (...args) => historyCalls.push(args), + }, + }; + + assert.equal(restoreRedirect(browserWindow), true); + assert.deepEqual(historyCalls, [[null, '', target]]); + assert.equal(storage.has(STORAGE_KEY), false); +}); + +test('rejects cross-origin, protocol-relative, and malformed stored targets', () => { + const rejected = [ + '//attacker.example/collect', + '/\\attacker.example/collect', + 'https://attacker.example/collect', + ['java', 'script:alert(1)'].join(''), + 'not a URL', + ]; + + rejected.forEach((target) => { + assert.equal(parseSameOriginTarget(target, 'https://runmprc.com'), null); + }); + assert.equal(parseSameOriginTarget('/safe?next=%2F%2Fexample.test', 'not an origin'), null); +}); + +test('clears an unsafe stored target without changing browser history', () => { + const storage = storageWith({ [STORAGE_KEY]: '//attacker.example/collect' }); + const browserWindow = { + location: { origin: 'https://runmprc.com' }, + sessionStorage: storage, + history: { replaceState: () => assert.fail('unsafe target must not be restored') }, + }; + + assert.equal(restoreRedirect(browserWindow), false); + assert.equal(storage.has(STORAGE_KEY), false); +}); + +test('fails safely when session storage is unavailable', () => { + const replacements = []; + const browserWindow = { + location: { + pathname: '/account', + search: '', + hash: '', + replace: (target) => replacements.push(target), + }, + sessionStorage: { + setItem: () => { throw new Error('storage blocked'); }, + }, + }; + + assert.equal(captureRedirect(browserWindow), false); + assert.deepEqual(replacements, ['/']); +}); + +test('fails safely when stored redirect state cannot be read', () => { + const browserWindow = { + location: { origin: 'https://runmprc.com' }, + sessionStorage: { + getItem: () => { throw new Error('storage blocked'); }, + removeItem: () => assert.fail('unreadable state must not be restored'), + }, + history: { replaceState: () => assert.fail('history must remain unchanged') }, + }; + + assert.equal(restoreRedirect(browserWindow), false); +}); + +test('clears temporary state and fails safely when history restoration throws', () => { + const storage = storageWith({ [STORAGE_KEY]: '/account' }); + const browserWindow = { + location: { origin: 'https://runmprc.com' }, + sessionStorage: storage, + history: { replaceState: () => { throw new Error('history blocked'); } }, + }; + + assert.equal(restoreRedirect(browserWindow), false); + assert.equal(storage.has(STORAGE_KEY), false); +}); + +test('the Pages documents invoke the shared capture and restore actions', () => { + const root = path.join(__dirname, '..', 'public'); + const notFound = fs.readFileSync(path.join(root, '404.html'), 'utf8'); + const index = fs.readFileSync(path.join(root, 'index.html'), 'utf8'); + + assert.match(notFound, /spa-navigation\.js" data-mprc-spa-action="capture"/); + assert.match(index, /spa-navigation\.js"[\s\S]*data-mprc-spa-action="restore"/); + assert.doesNotMatch(notFound, /sessionStorage\.setItem/); + assert.doesNotMatch(index, /sessionStorage\.getItem/); +}); From be3d2b10010873546467b69bb8c167dc238e2560 Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 00:34:29 -0700 Subject: [PATCH 2/9] docs: explain SAFETY-001 boundaries and operation --- AGENTS.md | 12 +++--- GITHUB_ISSUES.md | 60 ++++++++++++++++++------------ GITHUB_ISSUE_SLICES.md | 3 +- OPERATIONS_RUNBOOK.md | 59 ++++++++++++++++++++++------- README.md | 23 ++++++++++-- SECURITY.md | 18 ++++----- SYSTEM_DESIGN.md | 6 +-- docs/officers/PUBLISH_AND_CHECK.md | 18 +++++---- 8 files changed, 129 insertions(+), 70 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5ad406c7..71bbc8f9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -78,7 +78,7 @@ Split an issue before implementation if it cannot be safely reviewed as one focu - `firestore.rules`: browser access boundary. Admin SDK bypasses it, so also review server IAM/code. - `tests/firestore-rules/`: required allow/deny tests for every rule change. - `.github/workflows/`: CI and deployment trust boundary. -- `public/404.html` and `public/index.html`: current GitHub Pages SPA fallback. `public/spa-navigation.js` is queued #99 work and is **NOT AVAILABLE YET** on `main`. +- `public/404.html`, `public/index.html`, and `public/spa-navigation.js`: current same-origin GitHub Pages SPA callback bridge and its shared tested logic. - `docs/`: historical/developer/content context. - `docs/officers/`: current plain-language officer procedures, maps, continuity, and emergency guidance. @@ -94,20 +94,18 @@ npm --prefix functions ci npm --prefix functions run lint npm --prefix functions run test:run -- --runInBand CI=true npm test -- --watchAll=false --runInBand +npm run test:spa-navigation npm run test:rules CI=true DISABLE_ESLINT_PLUGIN=true npx --no-install react-scripts build ``` The Rules suite needs Java 17. The direct `react-scripts build` command is preferred for a diagnostic build because normal `npm run build` regenerates the sitemap. Run dependency audits for security/dependency issues, but do not apply forced automatic upgrades. -The following are **NOT AVAILABLE YET** on `main`: - -- `npm run emulators` and fail-closed Auth/Firestore/Functions isolation — owned by #99. -- `npm run test:spa-navigation` — owned by #99. - The deterministic frontend Jest baseline is available through the command above. Hosted CI runs that command as the blocking `Run frontend Jest tests` step under #124. This proves the suite ran for that commit; it does not prove branch protection, website publication, Firebase deployment, provider configuration, or production behavior. The remaining fail-closed lint and deployment work stays open under #105. -Do not run Firebase-backed local development against the current production-configured client. Do not describe queued working-tree commands as merged or available. +For Firebase-backed local development, run `npm run emulators` first and wait until Auth, Firestore, and Functions are ready under `demo-mprc-local`. In a second terminal, run `npm start` and use `http://localhost:3000`. Stop if any browser Firebase request uses a non-loopback host. The emulator suite does not isolate Stripe, Strava, email, or other provider calls made by Functions; those flows remain forbidden until their test configuration and safe sink are separately proven. + +Optimized builds and current deploy previews still use production Firebase configuration. Do not sign in, open private/admin pages, or exercise Firebase/provider behavior in a preview. Safe staging and provider isolation remain **NOT AVAILABLE YET** under #105/CONFIG. For Stripe lifecycle work, tests must include relevant negative and retry cases: invalid signature, duplicate/out-of-order Event, unpaid completion, amount/currency/environment mismatch, terminal-state protection, async success/failure, expiration, cancellation, and refund retry. Capacity/inventory work must include concurrent final-unit tests. diff --git a/GITHUB_ISSUES.md b/GITHUB_ISSUES.md index 00c747b2..0e46d080 100644 --- a/GITHUB_ISSUES.md +++ b/GITHUB_ISSUES.md @@ -49,10 +49,10 @@ Every issue inherits `AGENTS.md` and the definition of done in `IMPLEMENTATION_P | Order | ID | Title | Priority | Size | Status | Depends on | | ---: | --- | --- | --- | --- | --- | --- | -| 1 | SAFETY-001 | Preserve commerce/OAuth callbacks and isolate local Firebase Functions | P0 | S | implemented_locally | — | -| 2 | CI-001 | Repair test gates and secure the deployment pipeline | P0 | L | partial: #103 baseline + #124 hosted Jest; remaining gates open | SAFETY-001 | +| 1 | SAFETY-001 | Preserve commerce/OAuth callbacks and isolate local Firebase Functions | P0 | S | source/test complete under #99; provider/live callback unproven | — | +| 2 | CI-001 | Repair test gates and secure the deployment pipeline | P0 | L | partial: #103 baseline + #124 hosted Jest; #126 SPA gate and remaining gates open | SAFETY-001 | | 3 | SUPPLY-001 | Remove vulnerable dependency chains and stage SDK/build upgrades | P0 | L | ready | CI-001 baseline | -| 4 | SEC-001 | Replace the Firestore admin catch-all with resource-specific rules | P0 | M | implemented_locally | — | +| 4 | SEC-001 | Replace the Firestore admin catch-all with resource-specific rules | P0 | M | source merged #123; Firebase live unproven | — | | 5 | CONFIG-001 | Fail closed on server environment and commerce configuration | P0 | M | ready | CI-001A recommended | | 6 | AUTH-001 | Require verified email for member and privileged claims | P0 | M | partial: #98 merged; backend live unproven; parent open | SEC-001 recommended | | 7 | AUTH-002 | Replace the legacy static-key membership synchronization endpoint | P0 | M | ready | AUTH-001 | @@ -84,7 +84,7 @@ Every issue inherits `AGENTS.md` and the definition of done in `IMPLEMENTATION_P ## SAFETY-001 — Preserve commerce/OAuth callbacks and isolate local Firebase Functions **Labels:** `priority:P0`, `type:security`, `type:reliability`, `area:web`, `area:firebase`, `size:S` -**Status:** `implemented_locally`; focused and integrated local checks pass, but review/merge/deploy evidence remains +**Status:** Source/test outcome delivered under [#99](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/99); provider configuration and production callback behavior remain unproven **Depends on:** None ### Problem @@ -95,8 +95,11 @@ GitHub Pages routes unknown SPA paths through `public/404.html`. The original br - Preserve same-origin pathname, search, and hash through the GitHub Pages root redirect. - Clear temporary redirect state before restoration and reject malformed, protocol-relative, or cross-origin targets. -- Instantiate the shared Firebase Functions client and connect it to `127.0.0.1:5001` only in development. -- Add focused tests for callback preservation/rejection and development-versus-production emulator connections. +- Use a fully synthetic Firebase configuration in development/test. +- Connect shared Auth, Firestore, and Functions clients to their loopback emulators and stop startup if connector setup fails. +- Route the existing direct CSV Function URL through the same local/production resolver. +- Keep App Check, Analytics, and Sentry off locally; do not initialize Sentry on an initial capability callback URL. +- Add focused callback, environment, failure, direct-URL, and monitoring tests. ### Acceptance criteria @@ -104,25 +107,33 @@ GitHub Pages routes unknown SPA paths through `public/404.html`. The original br - [x] `/account/strava/callback?code=x&state=y` restores query parameters. - [x] Cross-origin and protocol-relative stored targets are discarded. - [x] Auth, Firestore, and Functions all connect to emulators in development. +- [x] Development/test Firebase configuration contains no production project identifiers. +- [x] App Check, Analytics, and Sentry do not initialize locally, even when public config is present. +- [x] Every Auth/Firestore/Functions connector failure stops local startup. +- [x] The direct CSV export resolves to the local Functions emulator outside production. - [x] None connects to an emulator in a production build. - [x] Production build contains the bridge and passes without changing unrelated sitemap content. +- [x] A demo-only CLI smoke reports Auth, Firestore, and Functions ready on the documented loopback ports. ### Verification -- Focused SPA navigation tests. -- Firebase resource environment tests with SDK calls mocked. -- Production compile invoked without sitemap-generating prebuild when only diagnosing. +- Node 20: 9/9 standalone SPA navigation tests. +- Node 20: 16/16 focused Firebase/Sentry tests and 4 frontend suites / 32 tests. +- Node 20: Functions lint and 17/17 Functions tests. +- Java 17 + Firebase CLI: demo-only Auth/Firestore/Functions readiness and loopback-port smoke. +- Diagnostic production compile invoked without the sitemap-generating prebuild. +- No production callback, account, data, provider call, Firebase deployment, or profile repair was tested. ### Agent handoff -Do not redesign routing or hosting in this issue; WEB-001 owns that. Do not include any real callback token in fixtures. +Do not redesign routing or hosting in this issue; WEB-001 owns that. Optimized previews still target production Firebase, so private preview behavior remains blocked on #105/CONFIG. Firebase emulators do not make Stripe, Strava, or email safe. Do not include any real callback token in fixtures. #118 still owns the reported profile failure. --- ## CI-001 — Repair test gates and secure the deployment pipeline **Labels:** `priority:P0`, `type:security`, `type:testing`, `area:ci`, `size:L`, `needs-external-config` -**Status:** Published as tracker [#105](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/105). [#103](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/103) merged the deterministic local frontend Jest baseline, and [#124](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/124) owns its named blocking hosted-CI step. Fail-closed lint, required branch protection, dependency/secret checks, and protected backend-first deployment remain open, so the delivery pipeline is still non-compliant overall. +**Status:** Published as tracker [#105](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/105). [#103](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/103) merged the deterministic local frontend Jest baseline, and [#124](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/124) added its named blocking hosted-CI step. [#126](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/126) owns the separate standalone SPA test gate. Fail-closed lint, required branch protection, dependency/secret checks, and protected backend-first deployment remain open, so the delivery pipeline is still non-compliant overall. **Depends on:** SAFETY-001 ### Problem @@ -164,7 +175,7 @@ Before #103, frontend tests failed because the Jest environment lacked `TextEnco ### Agent handoff -Repository changes can implement tests/workflow shape, but OIDC/IAM/environment protection requires an authorized owner. CI-001A/#103 owns test reliability, CI-001B1/#124 owns the hosted frontend-Jest step, and the remaining CI-001B2 deployment-identity/protection work must stay in separately claimed children. +Repository changes can implement tests/workflow shape, but OIDC/IAM/environment protection requires an authorized owner. CI-001A/#103 owns test reliability, CI-001B1/#124 owns the hosted frontend-Jest step, CI-001B1A/#126 owns the standalone SPA step, and the remaining CI-001B2 deployment-identity/protection work must stay in separately claimed children. --- @@ -214,7 +225,7 @@ Never use `npm audit fix --force`. Do not combine all upgrades in one unreviewab ## SEC-001 — Replace the Firestore admin catch-all with resource-specific rules **Labels:** `priority:P0`, `type:security`, `area:firebase`, `size:M` -**Status:** `implemented_locally`; rule and denial tests exist, with final review/deploy evidence still required +**Status:** Source and tests merged through [PR #123](https://github.com/Run-MPRC/Run-MPRC.github.io/pull/123) at `a7fc301e85b0aeabe396e771faea21d3fc8e7b2b`; Firebase deployment/live behavior remains unproven under #105 **Depends on:** None ### Problem @@ -234,19 +245,20 @@ Never use `npm audit fix --force`. Do not combine all upgrades in one unreviewab ### Acceptance criteria -- [ ] Admin client cannot read `members/{uid}/secrets/*`. -- [ ] Admin client cannot create/update/delete registration/order payment state or audit data. -- [ ] Admin client cannot read/write an unmatched arbitrary collection. -- [ ] Admin client cannot create `mail`, modify rate limits, webhook ledgers, or authorization/audit records. -- [ ] Current admin event/product list/editor and member/order/registration read views retain only required access. -- [ ] Public, member, unverified, and owner/self-service behavior remains explicitly tested. -- [ ] Rules deploy/compile and all emulator tests pass. +- [x] Admin client cannot read `members/{uid}/secrets/*` in the tested source Rules. +- [x] Admin client cannot create/update/delete registration/order payment state or audit data. +- [x] Admin client cannot read/write an unmatched arbitrary collection. +- [x] Admin client cannot create `mail`, modify rate limits, webhook ledgers, or authorization/audit records. +- [x] Current admin event/product list/editor and member/order/registration read views retain only required access. +- [x] Public, member, unverified, and owner/self-service behavior remains explicitly tested. +- [x] Rules compile in the emulator and all 295 Rules tests pass on Node 20/Java 17. +- [ ] #105 deploys the exact revision to protected staging and proves synthetic allow/deny plus rollback before production. ### Tests/evidence -- Add deny tests for every protected collection and collection-group query. -- Test admin read versus write separately. -- Test legacy event drafts are not publicly readable. +- [PR #123](https://github.com/Run-MPRC/Run-MPRC.github.io/pull/123) contains protected collection/collection-group, admin read/write, hostile catalog, owner-profile, and legacy-draft coverage. +- Two independent exact-commit reviews approved with no P0/P1/P2 findings. +- The merge workflow explicitly skipped Firebase because deployment authority was absent; green CI is source evidence only. ### Agent handoff @@ -1280,7 +1292,7 @@ The program is partially published. Treat unchecked rows as future governance wo - [ ] Create labels and milestones above. - [ ] Create only dependency-ready, non-duplicate issues after searching the live milestone; never bulk-publish the snapshot. - [ ] Add dependency links and project-board status. -- [x] Mark SAFETY-001, SEC-001, PAY-003, and PROMO-001 according to the verified working-tree result while keeping merge/deploy/full-acceptance gaps open. +- [x] Mark SAFETY-001, SEC-001, PAY-003, and PROMO-001 with distinct source, review, merge, deployment, provider, and live-verification states. - [ ] Assign business-owner issues to humans; do not assign LEGAL-001/OPS-001 solely to a coding agent. - [ ] Link root design documents from every issue and link GitHub issue URLs back into this file. - [ ] Do not include secrets, private account IDs, customer data, or confidential legal advice in issue bodies/evidence. diff --git a/GITHUB_ISSUE_SLICES.md b/GITHUB_ISSUE_SLICES.md index ab765464..355f3c99 100644 --- a/GITHUB_ISSUE_SLICES.md +++ b/GITHUB_ISSUE_SLICES.md @@ -17,7 +17,8 @@ An agent-ready child has one observable outcome, one trust boundary, explicit de | ID | Status/dependency | Bounded deliverable | Close evidence | | --- | --- | --- | --- | | CI-001A | merged/closed as #103 | Repair Jest browser globals and replace the stale starter test with a deterministic, no-provider-network MPRC app smoke test. | Node 20 focused/full frontend tests, changed-file lint, clean diff, and diagnostic build pass. | -| CI-001B1 | published as #124; CI-001A | Run the complete committed frontend Jest suite as a named blocking step in the hosted frontend CI job; update exact officer/engineering truth. | Local Node 20 suite/build plus exact pull-request and post-merge hosted step evidence; no production/provider claim. | +| CI-001B1 | merged/closed as #124; CI-001A | Run the complete committed frontend Jest suite as a named blocking step in the hosted frontend CI job; update exact officer/engineering truth. | Exact pull-request and post-merge hosted steps passed; Firebase skipped and Netlify exact production commit remained unverified. | +| CI-001B1A | published as #126; #99 and CI-001B1 | Run the standalone same-origin SPA callback suite as a separate blocking hosted frontend step. | Node 20 command plus deliberate failure proof on exact pull-request/post-merge commits; no provider callback claim. | | CI-001B2 | owner_action; CI-001B1 | Protect backend-first staging/production deployment; make lint/remaining checks fail closed; pin tools/Actions; remove frontend service-account JSON; configure OIDC/WIF, approvals, missing-config failure, smoke and roll-forward. | Required PR gate and negative credential run; private two-owner IAM review; staging deployment evidence. | | SUPPLY-001A | ready; CI-001A | Prove `gpxparser` unused and remove its obsolete transitive chain. | `rg`/tree proof, before/after audits, root lockfile diff, full frontend checks. | | SUPPLY-001B | ready; CI-001A | Patch React Router within compatible 6.x and resolve/test future flags without route redesign. | Navigation/callback tests, audit delta, build. | diff --git a/OPERATIONS_RUNBOOK.md b/OPERATIONS_RUNBOOK.md index 144377d4..34532554 100644 --- a/OPERATIONS_RUNBOOK.md +++ b/OPERATIONS_RUNBOOK.md @@ -46,7 +46,7 @@ Maintain an internal inventory with project/account IDs and console links: | Environment | Firebase project | Stripe mode/account | Web domain | GitHub environment | Status | | --- | --- | --- | --- | --- | --- | -| Local | Emulator project ID | Test + Stripe CLI | `localhost:3000` | None | **NOT AVAILABLE YET** — blocked on #99 fail-closed emulator isolation | +| Local source runtime | `demo-mprc-local` emulators | Providers remain separately gated | `localhost:3000` | None | Available for synthetic Firebase work after all three loopback emulators report ready | | CI | Demo/emulator project | Fixtures/test only | None | CI | Partially implemented | | Staging | **Create dedicated project** | Test/sandbox + unique endpoint | `dev.runmprc.com` | `staging` | Launch blocker | | Production | `mid-peninsula-running-club` (verify) | Live + unique endpoint | `runmprc.com` | `production` | Informational site only until gates close | @@ -139,33 +139,61 @@ STRAVA_CLIENT_ID=test-or-development-id STRAVA_CLIENT_SECRET=development-secret ``` -### Start emulators and app — NOT AVAILABLE YET +### Start emulators and app — available for synthetic Firebase work -Issue [#99](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/99) owns the demo-project script, Auth/Firestore/Functions emulator wiring, local monitoring isolation, and fail-closed startup. Those changes exist only in queued working-tree work and are not part of the current `main` baseline. +**Purpose:** start the browser against the local Auth, Firestore, and Functions emulators without a production Firebase fallback. -Until #99 merges, do not start the app for Firebase-backed account, admin, member, event, shop, payment, or private-data testing. The current client can inherit production Firebase configuration. Historical commands are not a safe workaround. +1. Confirm the terminal is at the repository root. +2. Confirm Node.js 20 is active. +3. In terminal 1, run: -After #99 merges, replace this warning with the exact merged commands and proof. The finished procedure must confirm the demo project and all three emulator connections before test data is created. +```bash +npm run emulators +``` + +4. Wait for the CLI to name project `demo-mprc-local`. +5. Wait for Auth on `127.0.0.1:9099`. +6. Wait for Firestore on `127.0.0.1:8080`. +7. Wait for Functions on `127.0.0.1:5001`. +8. Stop if any emulator fails, uses another project, or reports a production resource. +9. In terminal 2, run: + +```bash +npm start +``` -### Forward Stripe events +10. Open only `http://localhost:3000`. +11. Use synthetic records only. +12. Stop if browser Firebase traffic uses a host other than `127.0.0.1` or `localhost`. -Use the Stripe CLI logged into the intended test workspace: +Expected result: the browser uses a fully synthetic Firebase configuration, Auth/Firestore/Functions use loopback, and App Check, Analytics, and Sentry remain off. A failed emulator-connection setup stops app initialization. + +The CLI readiness messages prove the three processes listened during that run. Mocked frontend tests prove endpoint selection, not process readiness. If an emulator later stops, requests fail locally; do not change the project or disable the guard. + +Optimized builds are different. Netlify previews and a locally served `build/` directory use `NODE_ENV=production` and currently target production Firebase. Use them only for public, read-only visual checks. Do not sign in or open account, member, admin, event-registration, shop-purchase, or provider flows. + +### Forward Stripe events — NOT AVAILABLE YET for end-to-end use + +Firebase emulation does not isolate Stripe, Strava, email, or any other provider called by a Function. Do not run provider-facing flows until the owning issue proves the correct test account/key, safe email sink, configuration fail-closed behavior, and synthetic fixtures. + +When that gate is complete, the intended Stripe test-mode command is: ```bash stripe listen --forward-to http://127.0.0.1:5001/demo-mprc-local/us-central1/stripeWebhook ``` -Copy the CLI's temporary webhook signing secret only into the ignored local secret override. Do not use the Dashboard production endpoint secret locally. +Copy the CLI's temporary test webhook signing secret only into the ignored local secret override. Do not use a Dashboard production endpoint secret locally. -Trigger or complete test-mode Checkout through the application when testing end-to-end. Synthetic webhook triggers are useful for individual handlers but do not replace a real test Checkout flow with metadata and totals. +Do not trigger Checkout from the current app merely because the Firebase emulators are running. PAY/CONFIG/TEST issues own the later test-mode rehearsal. ### Local safety checks -- Browser/network calls target localhost for Auth, Firestore, and Functions. -- Stripe objects show test mode. +- The CLI names `demo-mprc-local` and all three expected loopback ports. +- Browser Firebase calls target only localhost/127.0.0.1. +- App Check, Analytics, and Sentry do not initialize locally. - No production customer email, address, DOB, or emergency-contact data is copied into local Firestore. -- Email extension is disabled or points to a safe sink/test mailbox. -- Sentry and analytics use development environments or are disabled. +- No checkout, refund, email, Strava, or other provider flow runs until its separate test configuration and safe sink are proven. +- No optimized preview/build is used for sign-in or private/Firebase behavior. ## 6. Verification commands @@ -175,9 +203,10 @@ Trigger or complete test-mode Checkout through the application when testing end- npm --prefix functions run lint npm --prefix functions run test:run -- --runInBand CI=true npm test -- --watchAll=false --runInBand +npm run test:spa-navigation ``` -The SPA navigation command is **NOT AVAILABLE YET** and belongs to #99. The frontend Jest command is a dependable local baseline and hosted CI runs it as the blocking `Run frontend Jest tests` step under [#124](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/124). Required branch protection, fail-closed lint, and protected deployment remain **NOT AVAILABLE YET** under [#105](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/105). +The frontend Jest command is a dependable local baseline and hosted CI runs it as the blocking `Run frontend Jest tests` step under [#124](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/124). The SPA command is a separate Node suite; hosted enforcement is still **NOT AVAILABLE YET** and belongs to [#126](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/126). Required branch protection, fail-closed lint, and protected deployment remain **NOT AVAILABLE YET** under [#105](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/105). ### Firestore Rules @@ -467,4 +496,6 @@ On the isolated #103 branch based at `0e03ac1`, Node 20 focused App tests pass 1 Issue #124 adds that complete committed frontend suite to the hosted frontend job as the named, blocking `Run frontend Jest tests` step. Its pull-request and post-merge runs are the proof that the exact commits executed Jest. Even a green step does not prove branch protection, website publication, Firebase deployment, provider configuration, or production behavior. +On the isolated #99 branch rebased over #124, Node 20 local evidence is: 9/9 standalone SPA tests; 16/16 focused Firebase/Sentry tests; 4 frontend suites / 32 tests; 17/17 Functions tests; a clean diagnostic build; and a demo-only CLI smoke in which Auth, Firestore, and Functions listened on the three documented loopback ports. No browser account flow, production callback, Stripe/Strava/email call, Firebase deployment, or production data was used. The standalone SPA suite is not yet hosted under #124; #126 owns that separate check. + Provider configuration, production secrets, Netlify publication, Firebase deployment, and live behavior always require separate dated evidence. Local tests and a green GitHub summary do not prove them. diff --git a/README.md b/README.md index eee7556b..8c9b2ce6 100644 --- a/README.md +++ b/README.md @@ -39,13 +39,13 @@ Historical developer/content/LLM guides remain under [`docs/`](./docs/README.md) - `firestore.rules` and `firestore.indexes.json`: browser data boundary and query indexes. - `tests/firestore-rules/`: emulator-based allow/deny coverage. - `.github/workflows/`: frontend, Functions, Rules CI and deployment automation. -- `public/404.html`: current GitHub Pages SPA fallback. The safer shared callback helper and its tests are **NOT AVAILABLE YET** and belong to #99. +- `public/404.html`, `public/index.html`, and `public/spa-navigation.js`: current tested GitHub Pages callback handoff. It preserves safe same-origin path, query, and fragment state. **Deployment reality checked 2026-07-12:** the repository publishes a GitHub Pages copy, while `runmprc.com` is currently served by a separate Netlify deployment. The Firebase workflow can remain green while skipping backend deployment when its service-account secret is absent. Treat website, Firebase, and provider deployment as separate states until CI-001 and hosting consolidation are complete. ## Local setup status -**Firebase-backed local development is NOT AVAILABLE YET on `main`.** Issue [#99](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/99) owns the missing demo-project emulator script, Auth/Firestore/Functions wiring, and fail-closed startup. Until #99 merges, `npm start` can use production Firebase configuration; do not use it for account, admin, member, event, shop, payment, or private-data testing. +The #99 local Firebase boundary is available for synthetic source development. It uses a non-addressable `demo-mprc-local` configuration and loopback Auth, Firestore, and Functions emulators. It stops startup when an emulator connection cannot be configured. App Check, Analytics, and Sentry stay off locally. Node.js 20 lockfile installation remains the baseline for maintainers preparing isolated, non-Firebase checks: @@ -54,7 +54,19 @@ npm ci --legacy-peer-deps npm --prefix functions ci ``` -After #99 merges, the runbook and this section must be updated from its merged tests rather than from the pre-merge working tree. +Start it in two terminals: + +```bash +# Terminal 1 +npm run emulators + +# Terminal 2 — only after all three emulators report ready +npm start +``` + +Open only `http://localhost:3000`. Stop if Firebase traffic uses a non-loopback host. Use synthetic records only. This does **not** make checkout, refunds, email, Strava, or other outside-provider calls safe; follow [OPERATIONS_RUNBOOK.md](./OPERATIONS_RUNBOOK.md) before any provider test. + +Do not use a Netlify preview or locally served optimized `build/` for sign-in, private pages, admin work, or Firebase testing. Those production-mode builds still target production Firebase until #105/CONFIG establishes staging. ## Verification @@ -62,11 +74,14 @@ After #99 merges, the runbook and this section must be updated from its merged t npm --prefix functions run lint npm --prefix functions run test:run -- --runInBand CI=true npm test -- --watchAll=false --runInBand +npm run test:spa-navigation npm run test:rules CI=true DISABLE_ESLINT_PLUGIN=true npx --no-install react-scripts build ``` -Rules tests require Java 17. The direct `react-scripts build` command is useful for a diagnostic compile because the normal `npm run build` runs the sitemap generator and may intentionally update `public/sitemap.xml`. The SPA navigation command remains queued under #99. The frontend Jest command provides the deterministic local baseline and runs as the blocking `Run frontend Jest tests` step in hosted CI under [#124](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/124). [#105](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/105) still owns non-mutating lint, branch/deployment protection, credentials, staging, and the other incomplete delivery gates. +Rules tests require Java 17. The direct `react-scripts build` command is useful for a diagnostic compile because the normal `npm run build` runs the sitemap generator and may intentionally update `public/sitemap.xml`. The frontend Jest command runs as the blocking `Run frontend Jest tests` step in hosted CI under [#124](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/124). The separate SPA callback command is locally available; [#126](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/126) owns adding it to hosted CI. [#105](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/105) still owns non-mutating lint, branch/deployment protection, credentials, staging, and the other incomplete delivery gates. + +These safety changes do not repair a missing member profile or prove deployed Firebase Rules/Functions. The reported profile-save failure remains [#118](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/118). ## Working on the platform diff --git a/SECURITY.md b/SECURITY.md index f26a2836..b2644499 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -43,11 +43,11 @@ The findings below describe the repository at the start of the 2026-07-12 assess | RISK-003 | Paid Checkout Sessions are created before their registration/order record is persisted. The webhook only queries for the stored Session ID. | A fast webhook or function crash can leave a paid Stripe transaction with no local confirmation. | Use a persistence-first, idempotent checkout saga and metadata-direct lookup. | | RISK-004 | Race capacity uses a count-then-create sequence outside a transaction; `registeredCount` is displayed but not maintained. | Concurrent users can exceed capacity; stale pending Sessions can block capacity for up to seven days. | Introduce transactional capacity counters/reservations, matching expiry, backfill, and concurrency tests. | | RISK-005 | Merchandise has product status but no SKU/variant inventory reservation. | The club can sell unavailable size/color combinations or oversell stock. | Add canonical variants and atomic inventory holds before live merchandise. | -| RISK-006 | Admin catch-all Firestore rule grants any `admin` browser read/write access to every document. It overrides the explicit deny on `members/*/secrets`, allows direct financial-state/audit mutation, and tests intentionally codify this. | A compromised admin session can read all Strava refresh tokens, rewrite orders/registrations, or erase audit evidence. | Remove the global catch-all; create explicit capability/resource rules; make secrets and financial state server-only even for browser admins. | +| RISK-006 | `main` source now removes the recursive browser-admin rule, but deployment repeatedly skipped Firebase and the live Rules revision is unproven. | A stale deployed ruleset may still expose secrets or financial/audit writes to a compromised admin session. | Use #105 to stage and deploy the exact tested #100 Rules revision, verify deny behavior with synthetic accounts, and record rollback. | | RISK-007 | Member promotion by email does not require the Firebase account's email to be verified. The legacy static-key bulk endpoint can grant `member` to a pre-registered known address. | An attacker who registers another member's email first can obtain membership access/discount after sync. | Require verified email and authoritative membership match; retire or strongly redesign the static-key endpoint. | | RISK-008 | `requireAppCheck` is controlled by optional `ENFORCE_APP_CHECK`; repository/deployment config does not prove it is true. Missing site key also disables client App Check. | Public callable functions may be scripted directly, enabling abuse and cloud/Stripe cost amplification. | Use Firebase runtime `enforceAppCheck: true` for sensitive callables, reCAPTCHA Enterprise, staged metrics, and no environment fail-open. | -| RISK-009 | GitHub Pages `404.html` stores only `pathname`; `index.html` restores only that value. Stripe and Strava deep-link callbacks lose query/hash data. | Paid success pages can lose Session/order/token references; OAuth loses `code`/`state`; users see failure despite successful external action. | Preserve same-origin path, search, and hash safely; add callback routing tests; prefer owned hosting rewrites. | -| RISK-010 | Local Firebase setup connects Auth and Firestore emulators but not the Functions emulator; every `getFunctions(app)` can target the deployed project. | Developers can accidentally invoke production checkout/admin/Strava functions while believing they are local. | Connect Functions emulator unconditionally in development and add environment safety tests. | +| RISK-009 | #99 source preserves and validates the complete same-origin Pages callback route, but no production Stripe/Strava callback has been rehearsed and the live custom domain uses a different host. | Hosting drift or an untested deployment can still break return links even when source tests pass. | Run the standalone callback suite in hosted CI under #126, verify test-mode callbacks only after provider gates, and consolidate hosting under WEB-001. | +| RISK-010 | Development/test now use synthetic loopback Auth, Firestore, and Functions, but optimized previews still target production Firebase and emulated Functions can call outside providers. | A maintainer can mistake a preview or Firebase emulator for complete staging and affect production/provider systems. | Prohibit private actions in previews; add dedicated staging/config under #105/CONFIG; prove test keys/sinks before provider flows. | | RISK-011 | Local cancellation only changes Firestore; it does not expire the active Stripe Session. The webhook can later move a cancelled record back to paid. | Customers can pay a supposedly cancelled registration/order; capacity/inventory and support state diverge. | Add explicit cancellation saga with Stripe Session expiry and allowed-state transitions. | | RISK-012 | Late registration creates reusable Stripe Payment Links, but records have no Session ID and the webhook searches only by Session/PaymentIntent. | Late payments do not reconcile; reuse can produce multiple charges/Sessions for one logical registration. | Replace with one-off idempotent Checkout Sessions or fully constrain/map Payment Links. | | RISK-013 | `allow_promotion_codes: true` permits Stripe discounts while Firestore stores base price and webhook does not validate/store discount/actual total. | Reporting, refund limits, and local payment confirmation can be wrong. | Disable promotions until a server-approved discount and total-reconciliation model exists. | @@ -87,20 +87,20 @@ The findings below describe the repository at the start of the 2026-07-12 assess | RISK-037 | Account/registration deletion, export, retention, backup, and restore procedures are incomplete. | Approve retention matrix, automate minimization, support access/deletion requests, and test backup restoration. | | RISK-038 | Source-controlled secret scan is ad hoc; no continuous secret scanner, dependency update bot, SBOM, provenance, or branch protection is documented. | Add secret/dependency/code scanning, reviewed lockfile updates, protected environments/branches, and artifact provenance appropriate to project scale. | -### Working-tree remediation ledger +### Source remediation ledger These entries are implementation evidence, not a production risk-acceptance decision: -| Risks | Working-tree result on 2026-07-12 | Remaining closure evidence | +| Risks | Source result | Remaining closure evidence | | --- | --- | --- | | RISK-001, RISK-002, RISK-034 | PAY-003 safety slice adds durable Stripe Event deduplication, transactional transition/processed marking, paid/amount/currency/Checkout-mode/reference-shape and explicitly configured livemode checks, zero-discount enforcement, async outcomes, monotonic refund/terminal guards, quarantine, generic signature failures, and a webhook-secret-only binding. | 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, deploy, and reconcile. | -| RISK-006, part of RISK-024/RISK-032 | SEC-001 removes the recursive browser-admin rule. OAuth secrets and server-owned financial/operational collections are denied; current catalog writes are narrowly allowlisted. | Review and deploy rules, capture deployed-rule evidence, then move remaining catalog mutation to scoped APIs under ADMIN-001. | -| RISK-009 | SAFETY-001 preserves same-origin path, query, and hash through the GitHub Pages fallback and rejects unsafe stored targets. | Review/deploy and run real test-mode Stripe/Strava callback smoke tests; WEB-001 still owns controlled hosting. | -| RISK-010 | SAFETY-001 uses a `demo-mprc-local` app, connects the shared Functions/Auth/Firestore instances to local emulators in development/test, fails startup on connection configuration errors, and adds environment-isolation tests. | Confirm the full emulator workflow and CI gate; no production invocation was performed in this pass. | +| 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. | +| RISK-009 | #99 preserves same-origin path, query, and fragment state through the Pages fallback, clears temporary state, rejects unsafe targets, and suppresses monitoring startup on initial capability callbacks. | Add the standalone suite to hosted CI under #126; verify exact website publication and later run approved test-mode Stripe/Strava callbacks; WEB-001 still owns controlled hosting. | +| RISK-010 | #99 uses a fully synthetic `demo-mprc-local` configuration, connects Functions/Auth/Firestore to loopback in development/test, routes the direct CSV Function URL locally, fails startup on connector errors, and completed a demo-only three-port CLI smoke. | Current optimized previews still target production; provider calls are not isolated by Firebase. Add protected staging/config and test-provider/sink evidence before end-to-end flows. | | Part of RISK-007 | AUTH-001A [#98](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/98) was merged through [PR #107](https://github.com/Run-MPRC/Run-MPRC.github.io/pull/107) as `ce22c110f2132b157bd8a0d43b065585e0b43cb5`. Source and focused tests reject unverified targets at the two existing role-grant endpoints. | The workflow skipped Firebase because `FIREBASE_SERVICE_ACCOUNT` was absent, so backend-live behavior is unproven. Remaining AUTH-001 guards, Rules, verification mirror/refresh, demotion propagation, audit, and authoritative membership work stay open. | | RISK-013 | PROMO-001 sets both current Checkout creators to `allow_promotion_codes: false`; the webhook quarantines nonzero discounts plus unconfigured tax/shipping totals. | Add exact creator-payload tests, inventory pre-change Sessions/provider promotion configuration, keep the guard through staging/deploy, and implement an authoritative discount/tax/shipping contract before ever enabling it. | | Part of RISK-024 | SEC-001 protects token documents from every browser client. | OAUTH-001 must still add transactional refresh/versioning, scope/account binding, disconnect/revocation, IAM/encryption decision, and redacted lifecycle audit. | -| Part of RISK-027 | Firebase Analytics and Sentry do not initialize in local/test runtime in the working tree, even if production-looking public config is present. | Approve hosted consent/redaction/retention/replay policy, environment-specific configuration, payload inspection, and deploy evidence under OBS/DATA work. | +| Part of RISK-027 | #99 keeps App Check, Firebase Analytics, and Sentry from initializing in local/test even when public provider configuration is present. Initial capability callback routes also fail closed before Sentry startup. | #111 still owns general hosted URL/event/log redaction, already-initialized SPA transitions, replay/consent/retention policy, payload inspection, and provider evidence. | | Part of RISK-035 | The Jest environment is repaired, the stale frontend smoke test is replaced, and #124 adds the complete frontend suite as a named blocking step in the hosted frontend CI job. | Keep exact hosted evidence on #124; make lint non-mutating/fail-closed, prove required branch checks, and add the broader TEST-001 suite. | ## 5. Threat model diff --git a/SYSTEM_DESIGN.md b/SYSTEM_DESIGN.md index 18997b18..85b49c8b 100644 --- a/SYSTEM_DESIGN.md +++ b/SYSTEM_DESIGN.md @@ -88,7 +88,7 @@ flowchart LR | Hosting | Netlify currently answers `runmprc.com`; GitHub Actions also publishes a separate Pages copy | `netlify.toml`, `.github/workflows/deploy.yml`, `public/404.html` | Split and drifting as verified 2026-07-12. A Pages success does not prove the live Netlify site changed. Hosting authority, preview, DNS, headers, and rollback remain open CI/WEB work. | | Email | Firestore `mail` outbox designed for the Firebase Trigger Email extension | `functions/sendConfirmationEmail.js` | Extension/provider deployment is unverified; outbox creation is not transactionally idempotent and HTML needs escaping. | | Observability | Optional Sentry and Firebase Analytics | `src/services/monitoring`, `src/services/analytics` | Local/test initialization is disabled by #99 source. Hosted redaction, replay, consent, retention, and provider configuration remain unverified under #111. | -| Third-party fitness | Strava OAuth tokens and statistics | `functions/strava.js`, `src/services/strava` | Functional prototype. Working-tree Rules deny browser token access, but transactional refresh, scopes/revocation, IAM/encryption decision, and audit remain OAUTH-001. | +| Third-party fitness | Strava OAuth tokens and statistics | `functions/strava.js`, `src/services/strava` | Functional prototype. The #100 source Rules deny browser token access, but Firebase deployment is unproven and transactional refresh, scopes/revocation, IAM/encryption decision, and audit remain OAUTH-001. | The repository workflow presents GitHub Pages plus Firebase as a release path, but public DNS and response headers show that the live custom domain is served by Netlify. The two copies currently contain different bundles. The Firebase job can also finish green while explicitly skipping deployment when `FIREBASE_SERVICE_ACCOUNT` is absent. Treat Pages publication, Netlify production, and Firebase deployment as separate evidence until one hosting/release authority is chosen. The App Engine synchronization script is another surface that must be documented as active or retired. @@ -326,12 +326,12 @@ flowchart LR Auth["Auth emulator\n127.0.0.1:9099"] Firestore["Firestore emulator\n127.0.0.1:8080"] Functions["Functions emulator\n127.0.0.1:5001"] - Providers["Stripe, Strava, email,\nand other outside providers"] + Providers["Stripe, Strava, email,\nand other outside providers\nNOT isolated by Firebase"] Browser --> Auth Browser --> Firestore Browser --> Functions - Functions -. "NOT isolated by Firebase" .-> Providers + Functions -.-> Providers ``` Text alternative: the local browser uses only the three loopback Firebase emulators. A Function can still call an outside provider, so Firebase emulator readiness alone does not authorize checkout, refund, email, or Strava testing. diff --git a/docs/officers/PUBLISH_AND_CHECK.md b/docs/officers/PUBLISH_AND_CHECK.md index fbde1c6a..29a9c8ae 100644 --- a/docs/officers/PUBLISH_AND_CHECK.md +++ b/docs/officers/PUBLISH_AND_CHECK.md @@ -18,6 +18,7 @@ As of **2026-07-13**: - The GitHub repository currently opens stale, unprotected `dev` by default; use the explicit `main` branch and do not merge `dev` into `main` as a shortcut. - GitHub Pages builds a copy of the website. - `runmprc.com` is currently served by Netlify, not that Pages copy. +- Current previews are optimized production-mode builds and can point at production Firebase. They are safe only for public, read-only page review. - The Firebase step can say success while skipping deployment when its service-account secret is absent. - Therefore, a green workflow does not prove the public site or backend changed. @@ -29,12 +30,13 @@ As of **2026-07-13**: 4. Open the `Frontend lint + build` job. 5. Confirm `Run frontend Jest tests` is present and green. Stop if it is missing, skipped, or failed. 6. Confirm the other required test steps are green. -7. Ask the platform maintainer for separate non-mutating lint evidence when lint applies. Do not use the green job as lint proof. -8. Confirm the officer guide was updated when needed. -9. Read the rollback note. -10. Confirm you understand that a merge automatically publishes the GitHub Pages copy. -11. Ask the platform maintainer whether any outside automation may also publish from the merge. -12. Do not merge if either automatic publication is unacceptable or unknown. +7. If you open a preview, review only public pages. Do not sign in, open a private/admin page, submit a form, or test a signup, checkout, refund, email, or Strava action. +8. Ask the platform maintainer for separate non-mutating lint evidence when lint applies. Do not use the green job as lint proof. +9. Confirm the officer guide was updated when needed. +10. Read the rollback note. +11. Confirm you understand that a merge automatically publishes the GitHub Pages copy. +12. Ask the platform maintainer whether any outside automation may also publish from the merge. +13. Do not merge if either automatic publication is unacceptable or unknown. ## Netlify publication — NOT AVAILABLE YET @@ -45,7 +47,7 @@ Before a backup officer can publish independently, a claimed hosting issue must 3. The connected GitHub repository and branch. 4. The event that starts a production deploy. 5. The build command and public configuration names. -6. A safe preview URL for the intended commit. +6. A safe preview URL for the intended commit, plus proof that private/Firebase actions use staging before anyone signs in. 7. Where Netlify displays the deployed commit. 8. How to restore the previous known-good deploy. 9. Which DNS records point `runmprc.com` to Netlify. @@ -73,7 +75,7 @@ The named Jest step is green for both the pull-request commit and the merged com ## Stop conditions -Stop and ask the platform maintainer if the pull request does not target `main`; approval or the rollback note is missing; the named Jest step is missing, skipped, or failed; publication is unexpected; the deployed commit is unknown; or any live surface disagrees with the merged change. +Stop and ask the platform maintainer if the pull request does not target `main`; approval or the rollback note is missing; the named Jest step is missing, skipped, or failed; a preview asks you to sign in or use private/Firebase behavior; publication is unexpected; the deployed commit is unknown; or any live surface disagrees with the merged change. ## Success proof From f97c2f4cc30bfda1de48b345133fb4c31ef8e5ba Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 00:41:03 -0700 Subject: [PATCH 3/9] fix: keep callback capabilities out of telemetry --- GITHUB_ISSUES.md | 9 +++++--- GITHUB_ISSUE_SLICES.md | 2 +- OPERATIONS_RUNBOOK.md | 2 +- SECURITY.md | 4 ++-- SYSTEM_DESIGN.md | 2 +- public/404.html | 1 + public/index.html | 1 + .../firebase/FirebaseResources.test.ts | 21 +++++++++++++++++- src/services/firebase/FirebaseResources.ts | 9 +++++++- src/services/monitoring/capabilityCallback.ts | 18 +++++++++++++++ src/services/monitoring/sentry.ts | 19 +++++----------- tests/spa-navigation.test.js | 22 +++++++++++++++++++ 12 files changed, 87 insertions(+), 23 deletions(-) create mode 100644 src/services/monitoring/capabilityCallback.ts diff --git a/GITHUB_ISSUES.md b/GITHUB_ISSUES.md index 0e46d080..bbae86dd 100644 --- a/GITHUB_ISSUES.md +++ b/GITHUB_ISSUES.md @@ -95,10 +95,11 @@ GitHub Pages routes unknown SPA paths through `public/404.html`. The original br - Preserve same-origin pathname, search, and hash through the GitHub Pages root redirect. - Clear temporary redirect state before restoration and reject malformed, protocol-relative, or cross-origin targets. +- Send only the site origin as the referrer when either Pages document loads a subresource. - Use a fully synthetic Firebase configuration in development/test. - Connect shared Auth, Firestore, and Functions clients to their loopback emulators and stop startup if connector setup fails. - Route the existing direct CSV Function URL through the same local/production resolver. -- Keep App Check, Analytics, and Sentry off locally; do not initialize Sentry on an initial capability callback URL. +- Keep App Check, Analytics, and Sentry off locally; do not initialize Analytics or Sentry on an initial capability callback URL. - Add focused callback, environment, failure, direct-URL, and monitoring tests. ### Acceptance criteria @@ -106,9 +107,11 @@ GitHub Pages routes unknown SPA paths through `public/404.html`. The original br - [x] `/register/success?session_id=cs_test_x®=r1#done` restores exactly after the 404 bridge. - [x] `/account/strava/callback?code=x&state=y` restores query parameters. - [x] Cross-origin and protocol-relative stored targets are discarded. +- [x] Both Pages documents apply `strict-origin` before their first subresource. - [x] Auth, Firestore, and Functions all connect to emulators in development. - [x] Development/test Firebase configuration contains no production project identifiers. - [x] App Check, Analytics, and Sentry do not initialize locally, even when public config is present. +- [x] Analytics and Sentry do not initialize on an initial capability callback carrying query or fragment state. - [x] Every Auth/Firestore/Functions connector failure stops local startup. - [x] The direct CSV export resolves to the local Functions emulator outside production. - [x] None connects to an emulator in a production build. @@ -117,8 +120,8 @@ GitHub Pages routes unknown SPA paths through `public/404.html`. The original br ### Verification -- Node 20: 9/9 standalone SPA navigation tests. -- Node 20: 16/16 focused Firebase/Sentry tests and 4 frontend suites / 32 tests. +- Node 20: 10/10 standalone SPA navigation/referrer-policy tests. +- Node 20: 20/20 focused Firebase/monitoring tests and 4 frontend suites / 36 tests. - Node 20: Functions lint and 17/17 Functions tests. - Java 17 + Firebase CLI: demo-only Auth/Firestore/Functions readiness and loopback-port smoke. - Diagnostic production compile invoked without the sitemap-generating prebuild. diff --git a/GITHUB_ISSUE_SLICES.md b/GITHUB_ISSUE_SLICES.md index 355f3c99..ac05a71a 100644 --- a/GITHUB_ISSUE_SLICES.md +++ b/GITHUB_ISSUE_SLICES.md @@ -18,7 +18,7 @@ An agent-ready child has one observable outcome, one trust boundary, explicit de | --- | --- | --- | --- | | CI-001A | merged/closed as #103 | Repair Jest browser globals and replace the stale starter test with a deterministic, no-provider-network MPRC app smoke test. | Node 20 focused/full frontend tests, changed-file lint, clean diff, and diagnostic build pass. | | CI-001B1 | merged/closed as #124; CI-001A | Run the complete committed frontend Jest suite as a named blocking step in the hosted frontend CI job; update exact officer/engineering truth. | Exact pull-request and post-merge hosted steps passed; Firebase skipped and Netlify exact production commit remained unverified. | -| CI-001B1A | published as #126; #99 and CI-001B1 | Run the standalone same-origin SPA callback suite as a separate blocking hosted frontend step. | Node 20 command plus deliberate failure proof on exact pull-request/post-merge commits; no provider callback claim. | +| CI-001B1A | published as #126; #99 and CI-001B1 | Run the standalone same-origin SPA callback suite as a separate blocking hosted frontend step. | Local deliberate-failure proof plus the named hosted step passing on the exact pull-request and post-merge commits; no provider callback claim. | | CI-001B2 | owner_action; CI-001B1 | Protect backend-first staging/production deployment; make lint/remaining checks fail closed; pin tools/Actions; remove frontend service-account JSON; configure OIDC/WIF, approvals, missing-config failure, smoke and roll-forward. | Required PR gate and negative credential run; private two-owner IAM review; staging deployment evidence. | | SUPPLY-001A | ready; CI-001A | Prove `gpxparser` unused and remove its obsolete transitive chain. | `rg`/tree proof, before/after audits, root lockfile diff, full frontend checks. | | SUPPLY-001B | ready; CI-001A | Patch React Router within compatible 6.x and resolve/test future flags without route redesign. | Navigation/callback tests, audit delta, build. | diff --git a/OPERATIONS_RUNBOOK.md b/OPERATIONS_RUNBOOK.md index 34532554..8f167a19 100644 --- a/OPERATIONS_RUNBOOK.md +++ b/OPERATIONS_RUNBOOK.md @@ -496,6 +496,6 @@ On the isolated #103 branch based at `0e03ac1`, Node 20 focused App tests pass 1 Issue #124 adds that complete committed frontend suite to the hosted frontend job as the named, blocking `Run frontend Jest tests` step. Its pull-request and post-merge runs are the proof that the exact commits executed Jest. Even a green step does not prove branch protection, website publication, Firebase deployment, provider configuration, or production behavior. -On the isolated #99 branch rebased over #124, Node 20 local evidence is: 9/9 standalone SPA tests; 16/16 focused Firebase/Sentry tests; 4 frontend suites / 32 tests; 17/17 Functions tests; a clean diagnostic build; and a demo-only CLI smoke in which Auth, Firestore, and Functions listened on the three documented loopback ports. No browser account flow, production callback, Stripe/Strava/email call, Firebase deployment, or production data was used. The standalone SPA suite is not yet hosted under #124; #126 owns that separate check. +On the isolated #99 branch rebased over #124, Node 20 local evidence is: 10/10 standalone SPA navigation/referrer-policy tests; 20/20 focused Firebase/monitoring tests; 4 frontend suites / 36 tests; 17/17 Functions tests; a clean diagnostic build; and a demo-only CLI smoke in which Auth, Firestore, and Functions listened on the three documented loopback ports. No browser account flow, production callback, Stripe/Strava/email call, Firebase deployment, or production data was used. The standalone SPA suite is not yet hosted under #124; #126 owns that separate check. Provider configuration, production secrets, Netlify publication, Firebase deployment, and live behavior always require separate dated evidence. Local tests and a green GitHub summary do not prove them. diff --git a/SECURITY.md b/SECURITY.md index b2644499..f47706da 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -46,7 +46,7 @@ The findings below describe the repository at the start of the 2026-07-12 assess | RISK-006 | `main` source now removes the recursive browser-admin rule, but deployment repeatedly skipped Firebase and the live Rules revision is unproven. | A stale deployed ruleset may still expose secrets or financial/audit writes to a compromised admin session. | Use #105 to stage and deploy the exact tested #100 Rules revision, verify deny behavior with synthetic accounts, and record rollback. | | RISK-007 | Member promotion by email does not require the Firebase account's email to be verified. The legacy static-key bulk endpoint can grant `member` to a pre-registered known address. | An attacker who registers another member's email first can obtain membership access/discount after sync. | Require verified email and authoritative membership match; retire or strongly redesign the static-key endpoint. | | RISK-008 | `requireAppCheck` is controlled by optional `ENFORCE_APP_CHECK`; repository/deployment config does not prove it is true. Missing site key also disables client App Check. | Public callable functions may be scripted directly, enabling abuse and cloud/Stripe cost amplification. | Use Firebase runtime `enforceAppCheck: true` for sensitive callables, reCAPTCHA Enterprise, staged metrics, and no environment fail-open. | -| RISK-009 | #99 source preserves and validates the complete same-origin Pages callback route, but no production Stripe/Strava callback has been rehearsed and the live custom domain uses a different host. | Hosting drift or an untested deployment can still break return links even when source tests pass. | Run the standalone callback suite in hosted CI under #126, verify test-mode callbacks only after provider gates, and consolidate hosting under WEB-001. | +| RISK-009 | #99 source preserves and validates the complete same-origin Pages callback route, applies `strict-origin` before subresources, and suppresses initial callback telemetry; no production Stripe/Strava callback has been rehearsed and the live custom domain uses a different host. | Hosting drift or an untested deployment can still break return links even when source tests pass. | Run the standalone callback suite in hosted CI under #126, verify test-mode callbacks only after provider gates, and consolidate hosting under WEB-001. | | RISK-010 | Development/test now use synthetic loopback Auth, Firestore, and Functions, but optimized previews still target production Firebase and emulated Functions can call outside providers. | A maintainer can mistake a preview or Firebase emulator for complete staging and affect production/provider systems. | Prohibit private actions in previews; add dedicated staging/config under #105/CONFIG; prove test keys/sinks before provider flows. | | RISK-011 | Local cancellation only changes Firestore; it does not expire the active Stripe Session. The webhook can later move a cancelled record back to paid. | Customers can pay a supposedly cancelled registration/order; capacity/inventory and support state diverge. | Add explicit cancellation saga with Stripe Session expiry and allowed-state transitions. | | RISK-012 | Late registration creates reusable Stripe Payment Links, but records have no Session ID and the webhook searches only by Session/PaymentIntent. | Late payments do not reconcile; reuse can produce multiple charges/Sessions for one logical registration. | Replace with one-off idempotent Checkout Sessions or fully constrain/map Payment Links. | @@ -100,7 +100,7 @@ These entries are implementation evidence, not a production risk-acceptance deci | Part of RISK-007 | AUTH-001A [#98](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/98) was merged through [PR #107](https://github.com/Run-MPRC/Run-MPRC.github.io/pull/107) as `ce22c110f2132b157bd8a0d43b065585e0b43cb5`. Source and focused tests reject unverified targets at the two existing role-grant endpoints. | The workflow skipped Firebase because `FIREBASE_SERVICE_ACCOUNT` was absent, so backend-live behavior is unproven. Remaining AUTH-001 guards, Rules, verification mirror/refresh, demotion propagation, audit, and authoritative membership work stay open. | | RISK-013 | PROMO-001 sets both current Checkout creators to `allow_promotion_codes: false`; the webhook quarantines nonzero discounts plus unconfigured tax/shipping totals. | Add exact creator-payload tests, inventory pre-change Sessions/provider promotion configuration, keep the guard through staging/deploy, and implement an authoritative discount/tax/shipping contract before ever enabling it. | | Part of RISK-024 | SEC-001 protects token documents from every browser client. | OAUTH-001 must still add transactional refresh/versioning, scope/account binding, disconnect/revocation, IAM/encryption decision, and redacted lifecycle audit. | -| Part of RISK-027 | #99 keeps App Check, Firebase Analytics, and Sentry from initializing in local/test even when public provider configuration is present. Initial capability callback routes also fail closed before Sentry startup. | #111 still owns general hosted URL/event/log redaction, already-initialized SPA transitions, replay/consent/retention policy, payload inspection, and provider evidence. | +| Part of RISK-027 | #99 keeps App Check, Firebase Analytics, and Sentry from initializing in local/test even when public provider configuration is present. Initial capability callback routes also fail closed before Firebase Analytics or Sentry startup. | #111 still owns general hosted URL/event/log redaction, already-initialized SPA transitions, replay/consent/retention policy, payload inspection, and provider evidence. | | Part of RISK-035 | The Jest environment is repaired, the stale frontend smoke test is replaced, and #124 adds the complete frontend suite as a named blocking step in the hosted frontend CI job. | Keep exact hosted evidence on #124; make lint non-mutating/fail-closed, prove required branch checks, and add the broader TEST-001 suite. | ## 5. Threat model diff --git a/SYSTEM_DESIGN.md b/SYSTEM_DESIGN.md index 85b49c8b..767e7d68 100644 --- a/SYSTEM_DESIGN.md +++ b/SYSTEM_DESIGN.md @@ -109,7 +109,7 @@ flowchart LR Restore -->|unsafe| Reject ``` -Text alternative: the 404 page temporarily carries the complete return route to the root page; the root page deletes that temporary value before accepting only a same-origin route. The bridge does not prove payment, OAuth state, or identity. Server/provider verification still decides the result. +Text alternative: the 404 page temporarily carries the complete return route to the root page; an early referrer policy keeps the path/query/fragment out of subresource request headers, and the root page deletes the temporary value before accepting only a same-origin route. Analytics and Sentry stay off on that initial capability-bearing callback. The bridge does not prove payment, OAuth state, or identity. Server/provider verification still decides the result. ## 4. Target deployment topology diff --git a/public/404.html b/public/404.html index 45218775..ccb99e53 100644 --- a/public/404.html +++ b/public/404.html @@ -2,6 +2,7 @@ + Redirecting... diff --git a/public/index.html b/public/index.html index 5fb169e4..1fd92068 100644 --- a/public/index.html +++ b/public/index.html @@ -2,6 +2,7 @@ + diff --git a/src/services/firebase/FirebaseResources.test.ts b/src/services/firebase/FirebaseResources.test.ts index 113dc33d..970b6839 100644 --- a/src/services/firebase/FirebaseResources.test.ts +++ b/src/services/firebase/FirebaseResources.test.ts @@ -52,12 +52,18 @@ function setNodeEnv(value: string | undefined) { }); } -function createResourcesFor(nodeEnv: string, siteKey?: string) { +function createResourcesFor( + nodeEnv: string, + siteKey?: string, + locationPath = '/', +) { const originalNodeEnv = process.env.NODE_ENV; const originalSiteKey = process.env.REACT_APP_RECAPTCHA_SITE_KEY; + const originalLocation = `${window.location.pathname}${window.location.search}${window.location.hash}`; setNodeEnv(nodeEnv); if (siteKey === undefined) delete process.env.REACT_APP_RECAPTCHA_SITE_KEY; else process.env.REACT_APP_RECAPTCHA_SITE_KEY = siteKey; + window.history.replaceState(null, '', locationPath); try { let resources: import('./FirebaseResources').default; @@ -68,6 +74,7 @@ function createResourcesFor(nodeEnv: string, siteKey?: string) { }); return resources!; } finally { + window.history.replaceState(null, '', originalLocation); setNodeEnv(originalNodeEnv); if (originalSiteKey === undefined) delete process.env.REACT_APP_RECAPTCHA_SITE_KEY; else process.env.REACT_APP_RECAPTCHA_SITE_KEY = originalSiteKey; @@ -177,6 +184,18 @@ describe('FirebaseResources environment isolation', () => { ); }); + test.each([ + '/account/strava/callback?code=example&state=example-state', + '/register/success?registration=r1&token=example-capability', + '/shop/purchase/success?order=o1&token=example-capability', + '/register/success#example-capability', + ])('production does not initialize Analytics on a capability callback: %s', (pathName) => { + createResourcesFor('production', 'configured-public-site-key', pathName); + + expect(mockIsAnalyticsSupported).not.toHaveBeenCalled(); + expect(mockGetAnalytics).not.toHaveBeenCalled(); + }); + 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 fb222e91..071468c2 100644 --- a/src/services/firebase/FirebaseResources.ts +++ b/src/services/firebase/FirebaseResources.ts @@ -15,6 +15,8 @@ import { getFunctions, } from 'firebase/functions'; +import hasCapabilityCallbackState from '../monitoring/capabilityCallback'; + const isLocalRuntime = process.env.NODE_ENV !== 'production'; const LOCAL_FIREBASE_PROJECT_ID = 'demo-mprc-local'; const FUNCTION_NAME_PATTERN = /^[A-Za-z][A-Za-z0-9_-]*$/; @@ -119,7 +121,12 @@ class FirebaseResources { private async initAnalytics(): Promise { // Analytics has no Emulator Suite target. Never initialize it from local // development or tests, even when a production measurement ID is present. - if (isLocalRuntime) return; + // A restored OAuth/checkout callback may contain a capability in its URL; + // do not let automatic analytics startup observe that initial location. + if ( + isLocalRuntime + || (typeof window !== 'undefined' && hasCapabilityCallbackState(window.location)) + ) return; try { const supported = await isSupported(); if (supported) { diff --git a/src/services/monitoring/capabilityCallback.ts b/src/services/monitoring/capabilityCallback.ts new file mode 100644 index 00000000..1676455a --- /dev/null +++ b/src/services/monitoring/capabilityCallback.ts @@ -0,0 +1,18 @@ +type BrowserLocationState = { + pathname: string; + search: string; + hash: string; +}; + +const CAPABILITY_CALLBACK_PATHS = new Set([ + '/account/strava/callback', + '/register/success', + '/shop/purchase/success', +]); + +export default function hasCapabilityCallbackState( + location: BrowserLocationState, +): boolean { + return CAPABILITY_CALLBACK_PATHS.has(location.pathname) + && (location.search.length > 0 || location.hash.length > 0); +} diff --git a/src/services/monitoring/sentry.ts b/src/services/monitoring/sentry.ts index 2f0c0690..a20e423c 100644 --- a/src/services/monitoring/sentry.ts +++ b/src/services/monitoring/sentry.ts @@ -1,5 +1,7 @@ import * as Sentry from '@sentry/react'; +import hasCapabilityCallbackState from './capabilityCallback'; + /** * Initialize Sentry error monitoring iff REACT_APP_SENTRY_DSN is set. * Without the DSN this is a no-op, so the build works fine without Sentry @@ -10,25 +12,16 @@ import * as Sentry from '@sentry/react'; */ let initialized = false; -const CAPABILITY_CALLBACK_PATHS = new Set([ - '/account/strava/callback', - '/register/success', - '/shop/purchase/success', -]); - -function hasCapabilityCallbackState(): boolean { - if (typeof window === 'undefined') return false; - const { pathname, search, hash } = window.location; - return CAPABILITY_CALLBACK_PATHS.has(pathname) - && (search.length > 0 || hash.length > 0); -} export function initSentry(): void { if (initialized) return; // Local/test sessions must not reach an outside monitoring service. In // production, do not initialize on callback URLs carrying OAuth or checkout // capabilities. #111 owns the broader hosted redaction/replay policy. - if (process.env.NODE_ENV !== 'production' || hasCapabilityCallbackState()) return; + if ( + process.env.NODE_ENV !== 'production' + || (typeof window !== 'undefined' && hasCapabilityCallbackState(window.location)) + ) return; const dsn = process.env.REACT_APP_SENTRY_DSN; if (!dsn) return; const environment = process.env.REACT_APP_SENTRY_ENV diff --git a/tests/spa-navigation.test.js b/tests/spa-navigation.test.js index f7f2060e..aad0afbe 100644 --- a/tests/spa-navigation.test.js +++ b/tests/spa-navigation.test.js @@ -147,3 +147,25 @@ test('the Pages documents invoke the shared capture and restore actions', () => assert.doesNotMatch(notFound, /sessionStorage\.setItem/); assert.doesNotMatch(index, /sessionStorage\.getItem/); }); + +test('the Pages documents strip callback details before loading subresources', () => { + const root = path.join(__dirname, '..', 'public'); + const documents = [ + fs.readFileSync(path.join(root, '404.html'), 'utf8'), + fs.readFileSync(path.join(root, 'index.html'), 'utf8'), + ]; + + documents.forEach((document) => { + const policyIndex = document.search( + //i, + ); + const firstSubresourceIndex = document.search(/<(?:script|link|img)\b/i); + + assert.notEqual(policyIndex, -1, 'strict-origin policy must be present'); + assert.notEqual(firstSubresourceIndex, -1, 'document must load a subresource'); + assert.ok( + policyIndex < firstSubresourceIndex, + 'referrer policy must precede every subresource', + ); + }); +}); From 6082bf7d0f526d2958564258706f528d378bfc5c Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 00:44:08 -0700 Subject: [PATCH 4/9] fix: reject normalized protocol-relative callbacks --- public/spa-navigation.js | 3 +++ tests/spa-navigation.test.js | 43 ++++++++++++++++++++---------------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/public/spa-navigation.js b/public/spa-navigation.js index 3e6ee634..47e8c480 100644 --- a/public/spa-navigation.js +++ b/public/spa-navigation.js @@ -43,6 +43,9 @@ const normalizedOrigin = new URL(origin).origin; const target = new URL(storedTarget, normalizedOrigin); if (target.origin !== normalizedOrigin) return null; + if (!target.pathname.startsWith('/') || target.pathname.startsWith('//')) { + return null; + } return target.pathname + target.search + target.hash; } catch (_error) { return null; diff --git a/tests/spa-navigation.test.js b/tests/spa-navigation.test.js index aad0afbe..d23fd9f4 100644 --- a/tests/spa-navigation.test.js +++ b/tests/spa-navigation.test.js @@ -11,6 +11,17 @@ const { restoreRedirect, } = require('../public/spa-navigation'); +const UNSAFE_STORED_TARGETS = [ + '//attacker.example/collect', + '/\\attacker.example/collect', + 'https://attacker.example/collect', + ['java', 'script:alert(1)'].join(''), + 'not a URL', + '/%2e%2e//attacker.example', + '/.//attacker.example', + '/a/..//attacker.example', +]; + function storageWith(initial = {}) { const values = new Map(Object.entries(initial)); return { @@ -68,30 +79,24 @@ test('restores the exact same-origin route and clears temporary state', () => { }); test('rejects cross-origin, protocol-relative, and malformed stored targets', () => { - const rejected = [ - '//attacker.example/collect', - '/\\attacker.example/collect', - 'https://attacker.example/collect', - ['java', 'script:alert(1)'].join(''), - 'not a URL', - ]; - - rejected.forEach((target) => { + UNSAFE_STORED_TARGETS.forEach((target) => { assert.equal(parseSameOriginTarget(target, 'https://runmprc.com'), null); }); assert.equal(parseSameOriginTarget('/safe?next=%2F%2Fexample.test', 'not an origin'), null); }); -test('clears an unsafe stored target without changing browser history', () => { - const storage = storageWith({ [STORAGE_KEY]: '//attacker.example/collect' }); - const browserWindow = { - location: { origin: 'https://runmprc.com' }, - sessionStorage: storage, - history: { replaceState: () => assert.fail('unsafe target must not be restored') }, - }; - - assert.equal(restoreRedirect(browserWindow), false); - assert.equal(storage.has(STORAGE_KEY), false); +test('clears unsafe stored targets without changing browser history', () => { + UNSAFE_STORED_TARGETS.forEach((target) => { + const storage = storageWith({ [STORAGE_KEY]: target }); + const browserWindow = { + location: { origin: 'https://runmprc.com' }, + sessionStorage: storage, + history: { replaceState: () => assert.fail('unsafe target must not be restored') }, + }; + + assert.equal(restoreRedirect(browserWindow), false); + assert.equal(storage.has(STORAGE_KEY), false); + }); }); test('fails safely when session storage is unavailable', () => { From 2d2458d924688ccb97e6f30ac7276ad885998098 Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 00:46:36 -0700 Subject: [PATCH 5/9] fix: match callback guards to router semantics --- GITHUB_ISSUES.md | 2 +- OPERATIONS_RUNBOOK.md | 2 +- .../firebase/FirebaseResources.test.ts | 19 +++++++++++++++++++ src/services/monitoring/capabilityCallback.ts | 6 +++++- src/services/monitoring/sentry.test.ts | 2 ++ 5 files changed, 28 insertions(+), 3 deletions(-) diff --git a/GITHUB_ISSUES.md b/GITHUB_ISSUES.md index bbae86dd..7e9cd108 100644 --- a/GITHUB_ISSUES.md +++ b/GITHUB_ISSUES.md @@ -121,7 +121,7 @@ GitHub Pages routes unknown SPA paths through `public/404.html`. The original br ### Verification - Node 20: 10/10 standalone SPA navigation/referrer-policy tests. -- Node 20: 20/20 focused Firebase/monitoring tests and 4 frontend suites / 36 tests. +- Node 20: 25/25 focused Firebase/monitoring tests and 4 frontend suites / 41 tests. - Node 20: Functions lint and 17/17 Functions tests. - Java 17 + Firebase CLI: demo-only Auth/Firestore/Functions readiness and loopback-port smoke. - Diagnostic production compile invoked without the sitemap-generating prebuild. diff --git a/OPERATIONS_RUNBOOK.md b/OPERATIONS_RUNBOOK.md index 8f167a19..36d8a979 100644 --- a/OPERATIONS_RUNBOOK.md +++ b/OPERATIONS_RUNBOOK.md @@ -496,6 +496,6 @@ On the isolated #103 branch based at `0e03ac1`, Node 20 focused App tests pass 1 Issue #124 adds that complete committed frontend suite to the hosted frontend job as the named, blocking `Run frontend Jest tests` step. Its pull-request and post-merge runs are the proof that the exact commits executed Jest. Even a green step does not prove branch protection, website publication, Firebase deployment, provider configuration, or production behavior. -On the isolated #99 branch rebased over #124, Node 20 local evidence is: 10/10 standalone SPA navigation/referrer-policy tests; 20/20 focused Firebase/monitoring tests; 4 frontend suites / 36 tests; 17/17 Functions tests; a clean diagnostic build; and a demo-only CLI smoke in which Auth, Firestore, and Functions listened on the three documented loopback ports. No browser account flow, production callback, Stripe/Strava/email call, Firebase deployment, or production data was used. The standalone SPA suite is not yet hosted under #124; #126 owns that separate check. +On the isolated #99 branch rebased over #124, Node 20 local evidence is: 10/10 standalone SPA navigation/referrer-policy tests; 25/25 focused Firebase/monitoring tests; 4 frontend suites / 41 tests; 17/17 Functions tests; a clean diagnostic build; and a demo-only CLI smoke in which Auth, Firestore, and Functions listened on the three documented loopback ports. No browser account flow, production callback, Stripe/Strava/email call, Firebase deployment, or production data was used. The standalone SPA suite is not yet hosted under #124; #126 owns that separate check. Provider configuration, production secrets, Netlify publication, Firebase deployment, and live behavior always require separate dated evidence. Local tests and a green GitHub summary do not prove them. diff --git a/src/services/firebase/FirebaseResources.test.ts b/src/services/firebase/FirebaseResources.test.ts index 970b6839..e781cd5e 100644 --- a/src/services/firebase/FirebaseResources.test.ts +++ b/src/services/firebase/FirebaseResources.test.ts @@ -189,6 +189,8 @@ describe('FirebaseResources environment isolation', () => { '/register/success?registration=r1&token=example-capability', '/shop/purchase/success?order=o1&token=example-capability', '/register/success#example-capability', + '/REGISTER/SUCCESS?registration=r1&token=example-capability', + '/shop/purchase/success/?order=o1&token=example-capability', ])('production does not initialize Analytics on a capability callback: %s', (pathName) => { createResourcesFor('production', 'configured-public-site-key', pathName); @@ -196,6 +198,23 @@ describe('FirebaseResources environment isolation', () => { expect(mockGetAnalytics).not.toHaveBeenCalled(); }); + test('ordinary production query pages initialize supported Analytics', async () => { + mockIsAnalyticsSupported.mockResolvedValueOnce(true); + + const resources = createResourcesFor( + 'production', + 'configured-public-site-key', + '/events?source=public-calendar', + ); + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + + expect(mockIsAnalyticsSupported).toHaveBeenCalledTimes(1); + expect(mockGetAnalytics).toHaveBeenCalledWith(resources.app); + expect(resources.analytics).toEqual({ name: 'test-analytics' }); + }); + test('rejects malformed direct Function names before building a URL', () => { const resources = createResourcesFor('development'); diff --git a/src/services/monitoring/capabilityCallback.ts b/src/services/monitoring/capabilityCallback.ts index 1676455a..9eea39e2 100644 --- a/src/services/monitoring/capabilityCallback.ts +++ b/src/services/monitoring/capabilityCallback.ts @@ -13,6 +13,10 @@ const CAPABILITY_CALLBACK_PATHS = new Set([ export default function hasCapabilityCallbackState( location: BrowserLocationState, ): boolean { - return CAPABILITY_CALLBACK_PATHS.has(location.pathname) + const normalizedPathname = location.pathname === '/' + ? '/' + : location.pathname.toLowerCase().replace(/\/+$/, ''); + + return CAPABILITY_CALLBACK_PATHS.has(normalizedPathname) && (location.search.length > 0 || location.hash.length > 0); } diff --git a/src/services/monitoring/sentry.test.ts b/src/services/monitoring/sentry.test.ts index 936381e8..e2463fbb 100644 --- a/src/services/monitoring/sentry.test.ts +++ b/src/services/monitoring/sentry.test.ts @@ -63,6 +63,8 @@ describe('Sentry environment and callback isolation', () => { '/register/success?registration=r1&token=example-capability', '/shop/purchase/success?order=o1&token=example-capability', '/register/success#example-capability', + '/REGISTER/SUCCESS?registration=r1&token=example-capability', + '/shop/purchase/success/?order=o1&token=example-capability', ])('does not initialize monitoring on a capability callback: %s', (locationPath) => { initializeFor('production', locationPath); expect(mockInit).not.toHaveBeenCalled(); From ea02198ac624b6ab590297f0340701a7ed256c9e Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 00:49:45 -0700 Subject: [PATCH 6/9] fix: decode callback paths like the router --- GITHUB_ISSUES.md | 2 +- OPERATIONS_RUNBOOK.md | 2 +- .../firebase/FirebaseResources.test.ts | 3 +++ src/services/monitoring/capabilityCallback.ts | 18 ++++++++++++++++-- src/services/monitoring/sentry.test.ts | 3 +++ 5 files changed, 24 insertions(+), 4 deletions(-) diff --git a/GITHUB_ISSUES.md b/GITHUB_ISSUES.md index 7e9cd108..1e13243c 100644 --- a/GITHUB_ISSUES.md +++ b/GITHUB_ISSUES.md @@ -121,7 +121,7 @@ GitHub Pages routes unknown SPA paths through `public/404.html`. The original br ### Verification - Node 20: 10/10 standalone SPA navigation/referrer-policy tests. -- Node 20: 25/25 focused Firebase/monitoring tests and 4 frontend suites / 41 tests. +- Node 20: 31/31 focused Firebase/monitoring tests and 4 frontend suites / 47 tests. - Node 20: Functions lint and 17/17 Functions tests. - Java 17 + Firebase CLI: demo-only Auth/Firestore/Functions readiness and loopback-port smoke. - Diagnostic production compile invoked without the sitemap-generating prebuild. diff --git a/OPERATIONS_RUNBOOK.md b/OPERATIONS_RUNBOOK.md index 36d8a979..dd6316cc 100644 --- a/OPERATIONS_RUNBOOK.md +++ b/OPERATIONS_RUNBOOK.md @@ -496,6 +496,6 @@ On the isolated #103 branch based at `0e03ac1`, Node 20 focused App tests pass 1 Issue #124 adds that complete committed frontend suite to the hosted frontend job as the named, blocking `Run frontend Jest tests` step. Its pull-request and post-merge runs are the proof that the exact commits executed Jest. Even a green step does not prove branch protection, website publication, Firebase deployment, provider configuration, or production behavior. -On the isolated #99 branch rebased over #124, Node 20 local evidence is: 10/10 standalone SPA navigation/referrer-policy tests; 25/25 focused Firebase/monitoring tests; 4 frontend suites / 41 tests; 17/17 Functions tests; a clean diagnostic build; and a demo-only CLI smoke in which Auth, Firestore, and Functions listened on the three documented loopback ports. No browser account flow, production callback, Stripe/Strava/email call, Firebase deployment, or production data was used. The standalone SPA suite is not yet hosted under #124; #126 owns that separate check. +On the isolated #99 branch rebased over #124, Node 20 local evidence is: 10/10 standalone SPA navigation/referrer-policy tests; 31/31 focused Firebase/monitoring tests; 4 frontend suites / 47 tests; 17/17 Functions tests; a clean diagnostic build; and a demo-only CLI smoke in which Auth, Firestore, and Functions listened on the three documented loopback ports. No browser account flow, production callback, Stripe/Strava/email call, Firebase deployment, or production data was used. The standalone SPA suite is not yet hosted under #124; #126 owns that separate check. Provider configuration, production secrets, Netlify publication, Firebase deployment, and live behavior always require separate dated evidence. Local tests and a green GitHub summary do not prove them. diff --git a/src/services/firebase/FirebaseResources.test.ts b/src/services/firebase/FirebaseResources.test.ts index e781cd5e..80a23b20 100644 --- a/src/services/firebase/FirebaseResources.test.ts +++ b/src/services/firebase/FirebaseResources.test.ts @@ -191,6 +191,9 @@ describe('FirebaseResources environment isolation', () => { '/register/success#example-capability', '/REGISTER/SUCCESS?registration=r1&token=example-capability', '/shop/purchase/success/?order=o1&token=example-capability', + '/register/%73uccess?registration=r1&token=example-capability', + '/%72egister/success#example-capability', + '/register/%ZZsuccess?token=example-capability', ])('production does not initialize Analytics on a capability callback: %s', (pathName) => { createResourcesFor('production', 'configured-public-site-key', pathName); diff --git a/src/services/monitoring/capabilityCallback.ts b/src/services/monitoring/capabilityCallback.ts index 9eea39e2..21ad99bc 100644 --- a/src/services/monitoring/capabilityCallback.ts +++ b/src/services/monitoring/capabilityCallback.ts @@ -13,9 +13,23 @@ const CAPABILITY_CALLBACK_PATHS = new Set([ export default function hasCapabilityCallbackState( location: BrowserLocationState, ): boolean { - const normalizedPathname = location.pathname === '/' + let routerPathname: string; + try { + // Match React Router's segment decoding without turning an encoded slash + // into a new path separator. + routerPathname = location.pathname + .split('/') + .map((segment) => decodeURIComponent(segment).replace(/\//g, '%2F')) + .join('/'); + } catch { + // A malformed encoded path cannot be compared reliably. If it also + // carries query/fragment state, suppress telemetry rather than guess. + return location.search.length > 0 || location.hash.length > 0; + } + + const normalizedPathname = routerPathname === '/' ? '/' - : location.pathname.toLowerCase().replace(/\/+$/, ''); + : routerPathname.toLowerCase().replace(/\/+$/, ''); return CAPABILITY_CALLBACK_PATHS.has(normalizedPathname) && (location.search.length > 0 || location.hash.length > 0); diff --git a/src/services/monitoring/sentry.test.ts b/src/services/monitoring/sentry.test.ts index e2463fbb..df78030b 100644 --- a/src/services/monitoring/sentry.test.ts +++ b/src/services/monitoring/sentry.test.ts @@ -65,6 +65,9 @@ describe('Sentry environment and callback isolation', () => { '/register/success#example-capability', '/REGISTER/SUCCESS?registration=r1&token=example-capability', '/shop/purchase/success/?order=o1&token=example-capability', + '/register/%73uccess?registration=r1&token=example-capability', + '/%72egister/success#example-capability', + '/register/%ZZsuccess?token=example-capability', ])('does not initialize monitoring on a capability callback: %s', (locationPath) => { initializeFor('production', locationPath); expect(mockInit).not.toHaveBeenCalled(); From 865143c6b0a8fa75eb23207b81c7275e50c0804a Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 00:53:03 -0700 Subject: [PATCH 7/9] fix: suppress App Check on capability callbacks --- GITHUB_ISSUES.md | 6 +++--- SECURITY.md | 4 ++-- SYSTEM_DESIGN.md | 2 +- src/services/firebase/FirebaseResources.test.ts | 4 +++- src/services/firebase/FirebaseResources.ts | 7 ++++++- 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/GITHUB_ISSUES.md b/GITHUB_ISSUES.md index 1e13243c..4e056290 100644 --- a/GITHUB_ISSUES.md +++ b/GITHUB_ISSUES.md @@ -99,7 +99,7 @@ GitHub Pages routes unknown SPA paths through `public/404.html`. The original br - Use a fully synthetic Firebase configuration in development/test. - Connect shared Auth, Firestore, and Functions clients to their loopback emulators and stop startup if connector setup fails. - Route the existing direct CSV Function URL through the same local/production resolver. -- Keep App Check, Analytics, and Sentry off locally; do not initialize Analytics or Sentry on an initial capability callback URL. +- Keep App Check, Analytics, and Sentry off locally; do not initialize App Check, Analytics, or Sentry on an initial capability callback URL. - Add focused callback, environment, failure, direct-URL, and monitoring tests. ### Acceptance criteria @@ -111,7 +111,7 @@ GitHub Pages routes unknown SPA paths through `public/404.html`. The original br - [x] Auth, Firestore, and Functions all connect to emulators in development. - [x] Development/test Firebase configuration contains no production project identifiers. - [x] App Check, Analytics, and Sentry do not initialize locally, even when public config is present. -- [x] Analytics and Sentry do not initialize on an initial capability callback carrying query or fragment state. +- [x] App Check, Analytics, and Sentry do not initialize on an initial capability callback carrying query or fragment state. - [x] Every Auth/Firestore/Functions connector failure stops local startup. - [x] The direct CSV export resolves to the local Functions emulator outside production. - [x] None connects to an emulator in a production build. @@ -129,7 +129,7 @@ GitHub Pages routes unknown SPA paths through `public/404.html`. The original br ### Agent handoff -Do not redesign routing or hosting in this issue; WEB-001 owns that. Optimized previews still target production Firebase, so private preview behavior remains blocked on #105/CONFIG. Firebase emulators do not make Stripe, Strava, or email safe. Do not include any real callback token in fixtures. #118 still owns the reported profile failure. +Do not redesign routing or hosting in this issue; WEB-001 owns that. Optimized previews still target production Firebase, so private preview behavior remains blocked on #105/CONFIG. Firebase emulators do not make Stripe, Strava, or email safe. ABUSE-001A/DATA-001A must design a safe handoff before a callback page depends on an App Check-protected browser request; do not weaken the initial capability guard. Do not include any real callback token in fixtures. #118 still owns the reported profile failure. --- diff --git a/SECURITY.md b/SECURITY.md index f47706da..75360fac 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -46,7 +46,7 @@ The findings below describe the repository at the start of the 2026-07-12 assess | RISK-006 | `main` source now removes the recursive browser-admin rule, but deployment repeatedly skipped Firebase and the live Rules revision is unproven. | A stale deployed ruleset may still expose secrets or financial/audit writes to a compromised admin session. | Use #105 to stage and deploy the exact tested #100 Rules revision, verify deny behavior with synthetic accounts, and record rollback. | | RISK-007 | Member promotion by email does not require the Firebase account's email to be verified. The legacy static-key bulk endpoint can grant `member` to a pre-registered known address. | An attacker who registers another member's email first can obtain membership access/discount after sync. | Require verified email and authoritative membership match; retire or strongly redesign the static-key endpoint. | | RISK-008 | `requireAppCheck` is controlled by optional `ENFORCE_APP_CHECK`; repository/deployment config does not prove it is true. Missing site key also disables client App Check. | Public callable functions may be scripted directly, enabling abuse and cloud/Stripe cost amplification. | Use Firebase runtime `enforceAppCheck: true` for sensitive callables, reCAPTCHA Enterprise, staged metrics, and no environment fail-open. | -| RISK-009 | #99 source preserves and validates the complete same-origin Pages callback route, applies `strict-origin` before subresources, and suppresses initial callback telemetry; no production Stripe/Strava callback has been rehearsed and the live custom domain uses a different host. | Hosting drift or an untested deployment can still break return links even when source tests pass. | Run the standalone callback suite in hosted CI under #126, verify test-mode callbacks only after provider gates, and consolidate hosting under WEB-001. | +| RISK-009 | #99 source preserves and validates the complete same-origin Pages callback route, applies `strict-origin` before subresources, and suppresses initial callback App Check/telemetry startup; no production Stripe/Strava callback has been rehearsed and the live custom domain uses a different host. | Hosting drift or an untested deployment can still break return links even when source tests pass. A future App Check-protected callback request needs a safe handoff instead of weakening this guard. | Run the standalone callback suite in hosted CI under #126, design protected callback handoff under ABUSE-001A/DATA-001A, verify test-mode callbacks only after provider gates, and consolidate hosting under WEB-001. | | RISK-010 | Development/test now use synthetic loopback Auth, Firestore, and Functions, but optimized previews still target production Firebase and emulated Functions can call outside providers. | A maintainer can mistake a preview or Firebase emulator for complete staging and affect production/provider systems. | Prohibit private actions in previews; add dedicated staging/config under #105/CONFIG; prove test keys/sinks before provider flows. | | RISK-011 | Local cancellation only changes Firestore; it does not expire the active Stripe Session. The webhook can later move a cancelled record back to paid. | Customers can pay a supposedly cancelled registration/order; capacity/inventory and support state diverge. | Add explicit cancellation saga with Stripe Session expiry and allowed-state transitions. | | RISK-012 | Late registration creates reusable Stripe Payment Links, but records have no Session ID and the webhook searches only by Session/PaymentIntent. | Late payments do not reconcile; reuse can produce multiple charges/Sessions for one logical registration. | Replace with one-off idempotent Checkout Sessions or fully constrain/map Payment Links. | @@ -100,7 +100,7 @@ These entries are implementation evidence, not a production risk-acceptance deci | Part of RISK-007 | AUTH-001A [#98](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/98) was merged through [PR #107](https://github.com/Run-MPRC/Run-MPRC.github.io/pull/107) as `ce22c110f2132b157bd8a0d43b065585e0b43cb5`. Source and focused tests reject unverified targets at the two existing role-grant endpoints. | The workflow skipped Firebase because `FIREBASE_SERVICE_ACCOUNT` was absent, so backend-live behavior is unproven. Remaining AUTH-001 guards, Rules, verification mirror/refresh, demotion propagation, audit, and authoritative membership work stay open. | | RISK-013 | PROMO-001 sets both current Checkout creators to `allow_promotion_codes: false`; the webhook quarantines nonzero discounts plus unconfigured tax/shipping totals. | Add exact creator-payload tests, inventory pre-change Sessions/provider promotion configuration, keep the guard through staging/deploy, and implement an authoritative discount/tax/shipping contract before ever enabling it. | | Part of RISK-024 | SEC-001 protects token documents from every browser client. | OAUTH-001 must still add transactional refresh/versioning, scope/account binding, disconnect/revocation, IAM/encryption decision, and redacted lifecycle audit. | -| Part of RISK-027 | #99 keeps App Check, Firebase Analytics, and Sentry from initializing in local/test even when public provider configuration is present. Initial capability callback routes also fail closed before Firebase Analytics or Sentry startup. | #111 still owns general hosted URL/event/log redaction, already-initialized SPA transitions, replay/consent/retention policy, payload inspection, and provider evidence. | +| Part of RISK-027 | #99 keeps App Check, Firebase Analytics, and Sentry from initializing in local/test even when public provider configuration is present. Initial capability callback routes also fail closed before App Check, Firebase Analytics, or Sentry startup. | #111 still owns general hosted URL/event/log redaction, already-initialized SPA transitions, replay/consent/retention policy, payload inspection, and provider evidence. ABUSE-001A/DATA-001A own safe protected callback handoff. | | Part of RISK-035 | The Jest environment is repaired, the stale frontend smoke test is replaced, and #124 adds the complete frontend suite as a named blocking step in the hosted frontend CI job. | Keep exact hosted evidence on #124; make lint non-mutating/fail-closed, prove required branch checks, and add the broader TEST-001 suite. | ## 5. Threat model diff --git a/SYSTEM_DESIGN.md b/SYSTEM_DESIGN.md index 767e7d68..ce6c03c2 100644 --- a/SYSTEM_DESIGN.md +++ b/SYSTEM_DESIGN.md @@ -109,7 +109,7 @@ flowchart LR Restore -->|unsafe| Reject ``` -Text alternative: the 404 page temporarily carries the complete return route to the root page; an early referrer policy keeps the path/query/fragment out of subresource request headers, and the root page deletes the temporary value before accepting only a same-origin route. Analytics and Sentry stay off on that initial capability-bearing callback. The bridge does not prove payment, OAuth state, or identity. Server/provider verification still decides the result. +Text alternative: the 404 page temporarily carries the complete return route to the root page; an early referrer policy keeps the path/query/fragment out of subresource request headers, and the root page deletes the temporary value before accepting only a same-origin route. App Check, Analytics, and Sentry stay off on that initial capability-bearing callback. The bridge does not prove payment, OAuth state, or identity. Server/provider verification still decides the result. ## 4. Target deployment topology diff --git a/src/services/firebase/FirebaseResources.test.ts b/src/services/firebase/FirebaseResources.test.ts index 80a23b20..2d830a9e 100644 --- a/src/services/firebase/FirebaseResources.test.ts +++ b/src/services/firebase/FirebaseResources.test.ts @@ -194,9 +194,11 @@ describe('FirebaseResources environment isolation', () => { '/register/%73uccess?registration=r1&token=example-capability', '/%72egister/success#example-capability', '/register/%ZZsuccess?token=example-capability', - ])('production does not initialize Analytics on a capability callback: %s', (pathName) => { + ])('production keeps App Check and Analytics off a capability callback: %s', (pathName) => { createResourcesFor('production', 'configured-public-site-key', pathName); + expect(mockReCaptchaV3Provider).not.toHaveBeenCalled(); + expect(mockInitializeAppCheck).not.toHaveBeenCalled(); expect(mockIsAnalyticsSupported).not.toHaveBeenCalled(); expect(mockGetAnalytics).not.toHaveBeenCalled(); }); diff --git a/src/services/firebase/FirebaseResources.ts b/src/services/firebase/FirebaseResources.ts index 071468c2..e8a2a6e4 100644 --- a/src/services/firebase/FirebaseResources.ts +++ b/src/services/firebase/FirebaseResources.ts @@ -75,7 +75,12 @@ 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. - if (isLocalRuntime) return; + // Its reCAPTCHA provider is also an outside script, so do not start it + // while an initial OAuth/checkout capability remains in the page URL. + if ( + isLocalRuntime + || (typeof window !== 'undefined' && hasCapabilityCallbackState(window.location)) + ) return; const siteKey = process.env.REACT_APP_RECAPTCHA_SITE_KEY; if (!siteKey) { From 3ef838008247dfc70126527b0d3f73b8c6212db6 Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 00:55:42 -0700 Subject: [PATCH 8/9] docs: gate App Check callback enforcement --- GITHUB_ISSUES.md | 7 +++++-- GITHUB_ISSUE_SLICES.md | 3 ++- SECURITY.md | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/GITHUB_ISSUES.md b/GITHUB_ISSUES.md index 4e056290..c5864d64 100644 --- a/GITHUB_ISSUES.md +++ b/GITHUB_ISSUES.md @@ -129,7 +129,7 @@ GitHub Pages routes unknown SPA paths through `public/404.html`. The original br ### Agent handoff -Do not redesign routing or hosting in this issue; WEB-001 owns that. Optimized previews still target production Firebase, so private preview behavior remains blocked on #105/CONFIG. Firebase emulators do not make Stripe, Strava, or email safe. ABUSE-001A/DATA-001A must design a safe handoff before a callback page depends on an App Check-protected browser request; do not weaken the initial capability guard. Do not include any real callback token in fixtures. #118 still owns the reported profile failure. +Do not redesign routing or hosting in this issue; WEB-001 owns that. Optimized previews still target production Firebase, so private preview behavior remains blocked on #105/CONFIG. Firebase emulators do not make Stripe, Strava, or email safe. ABUSE-001A must explicitly defer App Check enforcement on `lookupRegistration`, `lookupOrder`, and `stravaExchangeCode` while the initial capability guard is active. DATA-001A owns the two payment-confirmation handoffs; OAUTH-001C owns the Strava exchange handoff. Do not weaken the guard. Do not include any real callback token in fixtures. #118 still owns the reported profile failure. --- @@ -486,6 +486,7 @@ App Check is optional on the client and enforced only by a custom `ENFORCE_APP_C - Configure reCAPTCHA Enterprise for each web environment and observe App Check metrics. - Replace custom fail-open checks with Firebase `enforceAppCheck: true` runtime options on sensitive callable functions. +- Inventory the exact callable group. Mark `lookupRegistration` and `lookupOrder` deferred until DATA-001A supplies a tested safe confirmation handoff. Mark `stravaExchangeCode` deferred until OAUTH-001C supplies a tested safe OAuth handoff. - Evaluate limited-use/replay-protected tokens for checkout/refund commands after measuring web support and latency. - Derive client address only from platform-trusted request metadata. - HMAC rate-limit identifiers with a bound rotating secret; do not store raw email/IP. @@ -496,6 +497,7 @@ App Check is optional on the client and enforced only by a custom `ENFORCE_APP_C ### Acceptance criteria - [ ] Hosted sensitive callables reject missing/invalid App Check without relying on an optional env toggle. +- [ ] The enforcement inventory explicitly excludes the three initial-callback callables until their named handoff dependency merges; a test prevents accidental early enforcement. - [ ] Local emulator/CI behavior is explicit and cannot silently route to production. - [ ] Rate-limit documents contain no raw email/IP and expire automatically. - [ ] One attacker cannot block a victim solely by repeatedly submitting the victim's known email; mitigation/policy is documented. @@ -505,12 +507,13 @@ App Check is optional on the client and enforced only by a custom `ENFORCE_APP_C ### Tests/evidence - Missing/invalid/valid App Check tests in staging. +- Source inventory tests for enforced versus deferred callables, plus callback regression tests before any deferred callable moves to enforcement. - Rate-limit boundary, concurrent transaction, expiry, HMAC rotation, and spoofed-header tests. - Private console evidence for Enterprise key/domain, enforcement, TTL, budget, and alerts. ### Agent handoff -App Check is not authentication. Do not remove Auth/capability checks or rely on it as the only bot/fraud control. +App Check is not authentication. Do not remove Auth/capability checks or rely on it as the only bot/fraud control. Do not enforce it on `lookupRegistration`, `lookupOrder`, or `stravaExchangeCode` while #99 suppresses the reCAPTCHA provider on an initial capability URL. Move each callable only after DATA-001A or OAUTH-001C proves a safe handoff and the callback regression passes. --- diff --git a/GITHUB_ISSUE_SLICES.md b/GITHUB_ISSUE_SLICES.md index ac05a71a..209fe961 100644 --- a/GITHUB_ISSUE_SLICES.md +++ b/GITHUB_ISSUE_SLICES.md @@ -38,7 +38,8 @@ An agent-ready child has one observable outcome, one trust boundary, explicit de | AUTH-003D | proposed; AUTH-003B/C | Migrate admin UI/routes/token refresh, prove parity, then remove legacy alias. | Route/capability/demotion tests and staged operator sign-off. | | OAUTH-001A | proposed; SEC-001, AUTH-003B | Implement server-only, transaction/version-safe Strava token refresh so rotated refresh tokens cannot be lost. | Concurrent refresh/retry/conflict emulator tests. | | OAUTH-001B | owner_action; OAUTH-001A | Minimize scopes, implement disconnect/provider revocation, record encryption decision, and add access/refresh/revoke audit. | Test-provider revoke/refresh evidence and approved threat decision. | -| 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. | Missing/invalid/valid tests plus staged metrics before enforcement. | +| OAUTH-001C | proposed; #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. | 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 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-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. | diff --git a/SECURITY.md b/SECURITY.md index 75360fac..052c73cf 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -46,7 +46,7 @@ The findings below describe the repository at the start of the 2026-07-12 assess | RISK-006 | `main` source now removes the recursive browser-admin rule, but deployment repeatedly skipped Firebase and the live Rules revision is unproven. | A stale deployed ruleset may still expose secrets or financial/audit writes to a compromised admin session. | Use #105 to stage and deploy the exact tested #100 Rules revision, verify deny behavior with synthetic accounts, and record rollback. | | RISK-007 | Member promotion by email does not require the Firebase account's email to be verified. The legacy static-key bulk endpoint can grant `member` to a pre-registered known address. | An attacker who registers another member's email first can obtain membership access/discount after sync. | Require verified email and authoritative membership match; retire or strongly redesign the static-key endpoint. | | RISK-008 | `requireAppCheck` is controlled by optional `ENFORCE_APP_CHECK`; repository/deployment config does not prove it is true. Missing site key also disables client App Check. | Public callable functions may be scripted directly, enabling abuse and cloud/Stripe cost amplification. | Use Firebase runtime `enforceAppCheck: true` for sensitive callables, reCAPTCHA Enterprise, staged metrics, and no environment fail-open. | -| RISK-009 | #99 source preserves and validates the complete same-origin Pages callback route, applies `strict-origin` before subresources, and suppresses initial callback App Check/telemetry startup; no production Stripe/Strava callback has been rehearsed and the live custom domain uses a different host. | Hosting drift or an untested deployment can still break return links even when source tests pass. A future App Check-protected callback request needs a safe handoff instead of weakening this guard. | Run the standalone callback suite in hosted CI under #126, design protected callback handoff under ABUSE-001A/DATA-001A, verify test-mode callbacks only after provider gates, and consolidate hosting under WEB-001. | +| RISK-009 | #99 source preserves and validates the complete same-origin Pages callback route, applies `strict-origin` before subresources, and suppresses initial callback App Check/telemetry startup; no production Stripe/Strava callback has been rehearsed and the live custom domain uses a different host. | Hosting drift or an untested deployment can still break return links even when source tests pass. Enforcing App Check too early on the three callback callables would also break confirmation or Strava exchange. | Run the standalone callback suite in hosted CI under #126; keep `lookupRegistration`/`lookupOrder` deferred until DATA-001A and `stravaExchangeCode` deferred until OAUTH-001C; verify test-mode callbacks only after provider gates; consolidate hosting under WEB-001. | | RISK-010 | Development/test now use synthetic loopback Auth, Firestore, and Functions, but optimized previews still target production Firebase and emulated Functions can call outside providers. | A maintainer can mistake a preview or Firebase emulator for complete staging and affect production/provider systems. | Prohibit private actions in previews; add dedicated staging/config under #105/CONFIG; prove test keys/sinks before provider flows. | | RISK-011 | Local cancellation only changes Firestore; it does not expire the active Stripe Session. The webhook can later move a cancelled record back to paid. | Customers can pay a supposedly cancelled registration/order; capacity/inventory and support state diverge. | Add explicit cancellation saga with Stripe Session expiry and allowed-state transitions. | | RISK-012 | Late registration creates reusable Stripe Payment Links, but records have no Session ID and the webhook searches only by Session/PaymentIntent. | Late payments do not reconcile; reuse can produce multiple charges/Sessions for one logical registration. | Replace with one-off idempotent Checkout Sessions or fully constrain/map Payment Links. | @@ -100,7 +100,7 @@ These entries are implementation evidence, not a production risk-acceptance deci | Part of RISK-007 | AUTH-001A [#98](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/98) was merged through [PR #107](https://github.com/Run-MPRC/Run-MPRC.github.io/pull/107) as `ce22c110f2132b157bd8a0d43b065585e0b43cb5`. Source and focused tests reject unverified targets at the two existing role-grant endpoints. | The workflow skipped Firebase because `FIREBASE_SERVICE_ACCOUNT` was absent, so backend-live behavior is unproven. Remaining AUTH-001 guards, Rules, verification mirror/refresh, demotion propagation, audit, and authoritative membership work stay open. | | RISK-013 | PROMO-001 sets both current Checkout creators to `allow_promotion_codes: false`; the webhook quarantines nonzero discounts plus unconfigured tax/shipping totals. | Add exact creator-payload tests, inventory pre-change Sessions/provider promotion configuration, keep the guard through staging/deploy, and implement an authoritative discount/tax/shipping contract before ever enabling it. | | Part of RISK-024 | SEC-001 protects token documents from every browser client. | OAUTH-001 must still add transactional refresh/versioning, scope/account binding, disconnect/revocation, IAM/encryption decision, and redacted lifecycle audit. | -| Part of RISK-027 | #99 keeps App Check, Firebase Analytics, and Sentry from initializing in local/test even when public provider configuration is present. Initial capability callback routes also fail closed before App Check, Firebase Analytics, or Sentry startup. | #111 still owns general hosted URL/event/log redaction, already-initialized SPA transitions, replay/consent/retention policy, payload inspection, and provider evidence. ABUSE-001A/DATA-001A own safe protected callback handoff. | +| Part of RISK-027 | #99 keeps App Check, Firebase Analytics, and Sentry from initializing in local/test even when public provider configuration is present. Initial capability callback routes also fail closed before App Check, Firebase Analytics, or Sentry startup. | #111 still owns general hosted URL/event/log redaction, already-initialized SPA transitions, replay/consent/retention policy, payload inspection, and provider evidence. ABUSE-001A must defer the three callback callables; DATA-001A and OAUTH-001C own their safe protected handoffs. | | Part of RISK-035 | The Jest environment is repaired, the stale frontend smoke test is replaced, and #124 adds the complete frontend suite as a named blocking step in the hosted frontend CI job. | Keep exact hosted evidence on #124; make lint non-mutating/fail-closed, prove required branch checks, and add the broader TEST-001 suite. | ## 5. Threat model From f7b63d401e89e4d259b28baec093aeab790cda62 Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 00:56:35 -0700 Subject: [PATCH 9/9] docs: assign Strava callback handoff --- GITHUB_ISSUES.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/GITHUB_ISSUES.md b/GITHUB_ISSUES.md index c5864d64..01794071 100644 --- a/GITHUB_ISSUES.md +++ b/GITHUB_ISSUES.md @@ -405,6 +405,7 @@ SEC-001 removes browser access to stored OAuth secrets, but that does not comple ### Scope - Keep authorization code exchange, access token use, refresh, and disconnect entirely server-side with strict owner/capability checks. +- Create the OAUTH-001C callback handoff before App Check is enforced on `stravaExchangeCode`; keep `code`/`state` out of third-party startup while preserving state verification and server-only exchange. - Store token version/provider athlete reference, scope set, expiry, last refresh outcome, and minimum operational metadata separately from public member profile. - Use transaction/compare-and-set or a lease/version protocol so concurrent refresh cannot lose a rotated refresh token. - Minimize approved Strava scopes and reject unexpected returned scope/account binding. @@ -417,6 +418,7 @@ SEC-001 removes browser access to stored OAuth secrets, but that does not comple - [ ] No browser/admin rule can read or write token material. - [ ] Concurrent refresh preserves the newest valid rotated token and returns one coherent result. - [ ] Exchange/refresh verifies account binding and exact allowed scopes. +- [ ] `stravaExchangeCode` remains excluded from App Check enforcement until OAUTH-001C proves the protected callback handoff; after cutover, missing/invalid/valid App Check behavior is tested. - [ ] Disconnect revokes provider access when possible and makes local reuse impossible. - [ ] Logs/audit contain no authorization code, access token, refresh token, or full secret document. - [ ] Service IAM and encryption decision have named owner, rationale, and review date. @@ -424,11 +426,12 @@ SEC-001 removes browser access to stored OAuth secrets, but that does not comple ### Tests/evidence - Concurrent refresh, stale-version, rotated-token, retry, provider rejection, scope mismatch, wrong-user, disconnect/reconnect, and redacted-log tests. +- OAUTH-001C encoded/case/trailing callback, wrong/replayed state, and App Check handoff tests without a real member token. - Test-provider or mocked revocation evidence; private IAM/encryption review for hosted closure. ### Agent handoff -Use OAUTH-001A/B from the atomic slices. Never call the real Strava API with a member token in tests and never migrate/export production token documents into the workspace. +Use OAUTH-001A/B/C from the atomic slices. A owns refresh concurrency, B owns scopes/revocation/governance, and C owns the initial protected callback handoff. Never call the real Strava API with a member token in tests and never migrate/export production token documents into the workspace. ---