From 050a30f9d47018f60a47a18e74c746b86bbc43c9 Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Sun, 11 Oct 2026 11:39:27 -0700 Subject: [PATCH] fix: require native App Check for profile recovery --- GITHUB_ISSUES.md | 2 + OPERATIONS_RUNBOOK.md | 2 + SECURITY.md | 2 + docs/officers/LOW_COST_COMMUNITY.md | 28 +++- .../ensureMemberProfile.appCheck.test.js | 155 ++++++++++++++++++ functions/ensureMemberProfile.js | 5 +- 6 files changed, 191 insertions(+), 3 deletions(-) create mode 100644 functions/ensureMemberProfile.appCheck.test.js diff --git a/GITHUB_ISSUES.md b/GITHUB_ISSUES.md index 27e9601..03bca18 100644 --- a/GITHUB_ISSUES.md +++ b/GITHUB_ISSUES.md @@ -535,6 +535,8 @@ Split claim migration, MFA/recent-auth, and UI route projection into separate PR ABUSE-001A1 changes only the browser provider class from reCAPTCHA v3 to `ReCaptchaEnterpriseProvider`. It preserves local/test and initial capability-callback shutdown, requires the existing public build variable, keeps token auto-refresh, and retains fixed redacted diagnostics. Its tests use a synthetic key and no provider network call. +ABUSE-001A2A [#695](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/695) is the first narrow native callable source slice: unconditional `enforceAppCheck: true` on `ensureMemberProfile`, with actual SDK request-wrapper missing/invalid/valid cases using synthetic verifiers and mocked stores. Auth, empty payload, create-once schema/claims and resource limits remain required. The background Auth trigger, shared helper and callback-dependent callables are untouched. There is no dependency/schema/provider/deployment change; local requests without attestation fail too. This does not finish ABUSE-001A2 or #136: real token verification, staging/browser behavior and protected release remain separate proof. See the [officer review](./docs/officers/LOW_COST_COMMUNITY.md#profile-recovery-app-attestation--source-only-not-available-yet). + This source slice does not configure an Enterprise key/domain policy, set a build variable, publish the website, exchange a live token, or enforce App Check on a Function. ABUSE-001A2 must separately inventory safe callables, preserve the DATA-001A/OAUTH-001C callback deferrals, add native runtime enforcement with emulator behavior, observe staged metrics, and prove missing/invalid/valid requests. #113/#133/#136/WEB-001 retain project, protected configuration, deployment, provider readback, and live verification. ### Problem diff --git a/OPERATIONS_RUNBOOK.md b/OPERATIONS_RUNBOOK.md index 3508d73..18ad3b3 100644 --- a/OPERATIONS_RUNBOOK.md +++ b/OPERATIONS_RUNBOOK.md @@ -1004,6 +1004,8 @@ Never use destructive Git or Firestore reset operations on production as a rollb ### Missing member profile repair — NOT AVAILABLE YET +ABUSE-001A2A [#695](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/695) makes the recovery callable's native App Check option unconditional. Do not use `ENFORCE_APP_CHECK=false` or an absent flag as a recovery or testing bypass. The signup Auth trigger and paired resource limits are unchanged. Installed-SDK request tests use synthetic token verifiers with no socket, credential or provider; they prove the wrapper denies missing/invalid attestation before profile work, not Google's token verification or a usable hosted backend. A local/emulated request without App Check also fails; use reviewed synthetic verification fixtures, never production fallback or disabling enforcement to make a local request pass. Before a hosted release, separately prove missing/invalid app requests are denied, valid App Check plus Auth can recover a profile, anonymous/invalid-auth and injected fields fail, and retries preserve existing data/claims. Read-only Function metadata alone does not prove this SDK-level request boundary. Officers use the [profile-attestation review](./docs/officers/LOW_COST_COMMUNITY.md#profile-recovery-app-attestation--source-only-not-available-yet). + COST-001 [#683](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/683) adds source-only resource limits for this pair: `minInstances: 0`, `maxInstances: 2`, `memory: '256MB'`, and `timeoutSeconds: 30`. The actual SDK endpoint metadata is tested. The backend verifier reads the v1 provider fields `minInstances`, `maxInstances`, `availableMemoryMb`, and `timeout`. The API defines minimum instances as a proto3 implicit-presence integer, so an omitted field represents zero; the summary records `minInstancesReadback: 'proto3-default-zero'` instead of claiming an explicit value was returned. Null, malformed or differing values and missing maximum/memory/timeout evidence are rejected. An unreadable or incomplete Function cannot pass through this normalization. Existing project/revision/Rules/trigger checks remain required. See the [API schema](https://github.com/googleapis/googleapis/blob/master/google/cloud/functions/v1/functions.proto) and [JSON presence semantics](https://protobuf.dev/programming-guides/json/#presence-and-default-values). No migration is needed: profile schema, create-once transactions, claims, App Check/auth behavior, and trigger identities are unchanged. Execution timeouts and saturation may produce uncertain/busy results; the existing recovery path must preserve already committed data. Test cold starts and a small synthetic concurrent burst in staging before approving these initial limits for production. Max instances is not a hard monetary ceiling and may be briefly exceeded by the provider. Budgets/eligible spend caps, artifact retention, minimal runtime IAM, callable App Check enforcement, and the Node 20 maintenance deadline must be handled and verified in the provider release. Do not deploy extra Functions to make unrelated screens work. See the no-terminal [low-cost community review](./docs/officers/LOW_COST_COMMUNITY.md). diff --git a/SECURITY.md b/SECURITY.md index 149b3ba..09b7be7 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -18,6 +18,8 @@ The dual-runtime run exposed a Node 22 error-stack accessor that remained writab ## 2. Reporting a vulnerability +**Profile callable source boundary, 2026-10-11:** ABUSE-001A2A [#695](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/695) builds only `ensureMemberProfile` with native `enforceAppCheck: true`, independent of `ENFORCE_APP_CHECK`. Real installed-SDK request-wrapper tests use synthetic token verifiers and mocked profile stores; missing/invalid app tokens are rejected before business work with the flag absent, false or true. Valid app requests still require valid Auth and an empty payload. No shared helper, Auth-created trigger, resource limit, dependency, callback-dependent callable, provider setting or deployed service changes. This is source evidence, not cryptographic token verification, an emulator browser flow, or hosted enforcement. Other sensitive callables and #113/#133/#136 deployment/authorization/provider proof remain open. + Do not disclose a suspected vulnerability, secret, customer record, payment reference, or exploit in a public GitHub issue. Until MPRC publishes a dedicated security address: 1. Use the private contact channel listed on the live MPRC contact page. diff --git a/docs/officers/LOW_COST_COMMUNITY.md b/docs/officers/LOW_COST_COMMUNITY.md index ae4350e..6b23ba8 100644 --- a/docs/officers/LOW_COST_COMMUNITY.md +++ b/docs/officers/LOW_COST_COMMUNITY.md @@ -73,6 +73,28 @@ The #683 source change configured no budget, spending cap, artifact-cleanup poli [Issue #691](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/691) prepares Node 22, the program that runs the server code. Google schedules the old Node 20 runtime to be taken out of service on October 30, 2026. The update keeps the same two operations and resource limits. It adds automated compatibility and made-up profile checks, but does not deploy anything or fix every dependency warning. Hosted Node 22 behavior is **NOT AVAILABLE YET**. +## Profile recovery app attestation — SOURCE ONLY; NOT AVAILABLE YET + +**Purpose:** make sure profile recovery accepts only a verified request from the club app, even if an environment setting is missing. App Check is Firebase's app-attestation check; it does not establish membership or officer permission. + +**Approver:** the named platform/security release approver. **Prerequisites:** reviewed [#695](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/695) source, existing release dependencies, exact deployed-source proof, and isolated staging with made-up identities only. The source change does not deploy a Function or change billing. + +1. Ask the maintainer for the exact source and passing request-boundary tests. +2. Confirm only profile recovery gained native enforcement; signup, role policy and other services were not widened. +3. Observe the maintainer's private staging check that a request without app attestation cannot read or create a profile. +4. Observe the corresponding invalid-attestation rejection. +5. Observe a made-up signed-in member with valid app attestation recover one pending profile. +6. Confirm anonymous/invalid-auth and injected-field requests fail, and retries preserve profiles and roles. +7. Record source, tests, merge, staging/provider and production states separately. + +**Expected result / success proof:** exact reviewed source plus staged denial and valid recovery evidence. Source tests use synthetic token verifiers, not real Google token verification; resource-setting readback alone is not request proof. Officers do not run commands, handle tokens, enter real member data or edit cloud settings. + +**Stop conditions:** an unverified request succeeds, valid app attestation stops requiring sign-in, a flag can disable the boundary, or hosted evidence is missing. A local request without attestation is expected to fail; do not fall back to production to get a successful test. + +**Undo:** hold an unverified release. Ask the maintainer for a compatible protected roll-forward or rollback that preserves native enforcement; do not disable App Check or edit a member record. + +**Escalation:** platform/security maintainer. A specialist remains necessary for provider setup, tests and deployment. + ## Review before a low-cost release **Prerequisites:** issues #683 and #691 and their reviewed pull requests; the exact release source and test results; a named release approver; a private billing/payment-owner decision; and a platform maintainer. The existing release dependencies still apply. Never put payment details, passwords, or member records in an issue. @@ -80,14 +102,15 @@ The #683 source change configured no budget, spending cap, artifact-cleanup poli ```mermaid flowchart LR Scope["Confirm useful community scope"] --> Source["Review paired limits and tests"] - Source --> Provider["Maintainer verifies actual settings and cost controls"] + Source --> App["Review native profile app-attestation boundary"] + App --> Provider["Maintainer verifies actual settings and cost controls"] Provider --> Quality["Check made-up accounts, cold start, burst and recovery"] Quality --> Decision{"All proofs and approvals present?"} Decision -- Yes --> Release["Use approved backend-first release"] Decision -- No --> Hold["Keep release on hold"] ``` -Text alternative: confirm the small scope, review source, verify provider controls, test quality with made-up accounts, and release only when every proof and approval is present. +Text alternative: confirm the small scope, review source and native profile app attestation, verify provider controls, test quality with made-up accounts, and release only when every proof and approval is present. 1. Confirm the public pages to keep and the two profile operations to deploy. 2. Confirm the release adds no payment, email, photo, or synchronization service. @@ -120,3 +143,4 @@ Text alternative: confirm the small scope, review source, verify provider contro - [Firebase Hosting usage](https://firebase.google.com/docs/hosting/usage-quotas-pricing) - [Firebase free-trial billing and expiry](https://firebase.google.com/support/faq/#pricing) - [Budget scope and notification settings](https://docs.cloud.google.com/billing/docs/how-to/budgets) +- [Firebase native callable App Check enforcement](https://firebase.google.com/docs/app-check/cloud-functions) diff --git a/functions/ensureMemberProfile.appCheck.test.js b/functions/ensureMemberProfile.appCheck.test.js new file mode 100644 index 0000000..c954f6c --- /dev/null +++ b/functions/ensureMemberProfile.appCheck.test.js @@ -0,0 +1,155 @@ +const { EventEmitter } = require('node:events'); + +// Keep the actual Functions SDK and callable request wrapper. Only the token +// verifier and business stores are synthetic: no socket, credential or cloud +// endpoint is used. This is not cryptographic or hosted-provider proof. +jest.mock('firebase-admin/app', () => ({ + getApp: () => ({ name: 'synthetic-admin-app' }), +})); +jest.mock('firebase-admin/auth', () => { + const verifyIdToken = jest.fn(); + return { getAuth: () => ({ verifyIdToken }) }; +}); +jest.mock('firebase-admin/app-check', () => { + const verifyToken = jest.fn(); + return { getAppCheck: () => ({ verifyToken }) }; +}); +jest.mock('firebase-admin', () => ({ + auth: jest.fn(() => ({ getUser: jest.fn().mockResolvedValue({ uid: 'synthetic-uid' }) })), +})); +jest.mock('./memberProfile', () => ({ + memberProfileExists: jest.fn(), + ensureMemberProfileDocument: jest.fn(), +})); +jest.mock('firebase-functions/logger', () => ({ + debug: jest.fn(), info: jest.fn(), warn: jest.fn(), error: jest.fn(), +})); + +const admin = require('firebase-admin'); +const { getAuth } = require('firebase-admin/auth'); +const { getAppCheck } = require('firebase-admin/app-check'); +const { memberProfileExists, ensureMemberProfileDocument } = require('./memberProfile'); +const { verifyToken } = getAppCheck(); +const { verifyIdToken } = getAuth(); + +async function request(handler, headers = {}, data = {}) { + const requestHeaders = { 'content-type': 'application/json', ...headers }; + const req = { + method: 'POST', body: { data }, headers: requestHeaders, + header: (name) => requestHeaders[name.toLowerCase()], + }; + const res = new EventEmitter(); + const responseHeaders = new Map(); + res.setHeader = (name, value) => responseHeaders.set(name.toLowerCase(), value); + res.getHeader = (name) => responseHeaders.get(name.toLowerCase()); + res.status = (status) => { res.statusCode = status; return res; }; + res.send = (body) => { res.body = body; res.emit('finish'); return res; }; + await handler(req, res); + return { status: res.statusCode, body: res.body }; +} + +describe('profile recovery native App Check request boundary', () => { + const saved = {}; + let handler; + + beforeAll(() => { + for (const key of ['ENFORCE_APP_CHECK', 'FIREBASE_DEBUG_MODE', 'FIREBASE_DEBUG_FEATURES']) { + saved[key] = process.env[key]; + delete process.env[key]; + } + // Do not let an emulator debug token-decoding shortcut substitute for the + // actual SDK verification branch exercised by the synthetic verifiers. + handler = require('./ensureMemberProfile').ensureMemberProfile; + }); + + afterAll(() => { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + }); + + beforeEach(() => { + jest.clearAllMocks(); + verifyIdToken.mockResolvedValue({ uid: 'synthetic-uid' }); + memberProfileExists.mockResolvedValue(false); + ensureMemberProfileDocument.mockResolvedValue(undefined); + verifyToken.mockImplementation(async (token) => { + if (token !== 'synthetic-valid-app-token') throw new Error('Synthetic verification rejection.'); + return { appId: 'synthetic-app-id' }; + }); + }); + + describe.each([undefined, 'false', 'true'])('optional environment flag %s', (flag) => { + beforeEach(() => { + if (flag === undefined) delete process.env.ENFORCE_APP_CHECK; + else process.env.ENFORCE_APP_CHECK = flag; + }); + + test.each([undefined, 'synthetic-invalid-app-token'])( + 'rejects an unverified app before profile work: %s', async (appToken) => { + const headers = { authorization: 'Bearer synthetic-auth-token' }; + if (appToken !== undefined) headers['x-firebase-appcheck'] = appToken; + const response = await request(handler, headers); + expect(response).toEqual({ + status: 401, + body: { error: { status: 'UNAUTHENTICATED', message: 'Unauthenticated' } }, + }); + expect(admin.auth).not.toHaveBeenCalled(); + expect(memberProfileExists).not.toHaveBeenCalled(); + expect(ensureMemberProfileDocument).not.toHaveBeenCalled(); + expect(verifyToken).toHaveBeenCalledTimes(appToken === undefined ? 0 : 1); + }, + ); + + test('verified app and authenticated caller retain profile recovery', async () => { + const response = await request(handler, { + authorization: 'Bearer synthetic-auth-token', + 'x-firebase-appcheck': 'synthetic-valid-app-token', + }); + expect(response).toEqual({ status: 200, body: { result: { ready: true } } }); + expect(verifyToken).toHaveBeenCalledWith('synthetic-valid-app-token'); + expect(verifyIdToken).toHaveBeenCalledWith('synthetic-auth-token'); + expect(memberProfileExists).toHaveBeenCalledWith('synthetic-uid'); + expect(ensureMemberProfileDocument).toHaveBeenCalledWith({ uid: 'synthetic-uid' }); + }); + + test('verified app does not replace authentication', async () => { + const response = await request(handler, { + 'x-firebase-appcheck': 'synthetic-valid-app-token', + }); + expect(response.status).toBe(401); + expect(response.body.error.status).toBe('UNAUTHENTICATED'); + expect(admin.auth).not.toHaveBeenCalled(); + expect(memberProfileExists).not.toHaveBeenCalled(); + expect(ensureMemberProfileDocument).not.toHaveBeenCalled(); + }); + + test('verified app cannot admit an invalid Auth token', async () => { + verifyIdToken.mockRejectedValue(new Error('Synthetic authentication rejection.')); + const response = await request(handler, { + authorization: 'Bearer synthetic-invalid-auth-token', + 'x-firebase-appcheck': 'synthetic-valid-app-token', + }); + expect(response).toEqual({ + status: 401, + body: { error: { status: 'UNAUTHENTICATED', message: 'Unauthenticated' } }, + }); + expect(admin.auth).not.toHaveBeenCalled(); + expect(memberProfileExists).not.toHaveBeenCalled(); + expect(ensureMemberProfileDocument).not.toHaveBeenCalled(); + }); + + test('verified app does not authorize injected profile fields', async () => { + const response = await request(handler, { + authorization: 'Bearer synthetic-auth-token', + 'x-firebase-appcheck': 'synthetic-valid-app-token', + }, { role: 'admin' }); + expect(response.status).toBe(400); + expect(response.body.error.status).toBe('INVALID_ARGUMENT'); + expect(admin.auth).not.toHaveBeenCalled(); + expect(memberProfileExists).not.toHaveBeenCalled(); + expect(ensureMemberProfileDocument).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/functions/ensureMemberProfile.js b/functions/ensureMemberProfile.js index 02af4f7..46ef231 100644 --- a/functions/ensureMemberProfile.js +++ b/functions/ensureMemberProfile.js @@ -14,7 +14,10 @@ function isEmptyRequest(data) { && Object.keys(data).length === 0; } -exports.ensureMemberProfile = functions.runWith(profileRuntime).https.onCall(async (data, context) => { +exports.ensureMemberProfile = functions.runWith({ + ...profileRuntime, + enforceAppCheck: true, +}).https.onCall(async (data, context) => { requireAppCheck(context); if (!context.auth) {