From 425e39dc84ddca4bb59a3b97b7f6e802f8806506 Mon Sep 17 00:00:00 2001 From: heznpc Date: Thu, 21 May 2026 21:59:22 +0900 Subject: [PATCH 1/3] fix(auth)!: require id_token iss claim, not just allowlist match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isSessionStillValid had `if (claims.iss && !ALLOWED_ISSUERS.has(claims.iss))`, which short-circuits when iss is missing entirely — an iss-less id_token slipped past the Google-only allowlist that the function's docstring explicitly promises to enforce. Threat model: an attacker who can write to SecureStore (rooted/jailbroken device, shared-keychain entitlement bug, hostile companion app) could craft an iss-less blob and the client would honour it. Real-world likelihood is low for any single user, but this is a starter template — the reference implementation propagates to every downstream fork, so shipping a broken boundary here amplifies the harm. Fix: require `typeof claims.iss === 'string'` AND allowlist membership. Regression coverage: 6 new tests in tests/auth-context.test.js exercising the issuer/expiry boundary directly (the original suite only fed a fixed Google-iss happy-path token, which is why the bug survived). Surfaced by an adversarial second-pass audit on 2026-05-21. Previously declared done by the same session that wrote the function — A3 (happy-path only) catch-rate gap. --- lib/auth-context.js | 9 +++++- tests/auth-context.test.js | 64 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/lib/auth-context.js b/lib/auth-context.js index ab4ee4d..2a128b8 100644 --- a/lib/auth-context.js +++ b/lib/auth-context.js @@ -72,6 +72,9 @@ const ALLOWED_ISSUERS = new Set([ * The token came from Google over TLS via PKCE, so this isn't a server-grade * check — it's the *client*'s self-defense against a stolen device replaying * an old SecureStore blob. Returns true when the session should be honoured. + * + * The `iss` claim is REQUIRED, not optional. A token missing `iss` is by + * definition not Google-issued, so accepting it would defeat the allowlist. */ export function isSessionStillValid(session, now = Date.now()) { const idToken = session?.tokens?.idToken; @@ -79,7 +82,11 @@ export function isSessionStillValid(session, now = Date.now()) { const claims = decodeIdToken(idToken); if (!claims) return false; if (typeof claims.exp !== 'number' || claims.exp * 1000 <= now) return false; - if (claims.iss && !ALLOWED_ISSUERS.has(claims.iss)) return false; + // Require iss AND match the allowlist. A missing iss must be rejected: + // an `iss`-less token cannot have come from Google's documented OIDC flow. + if (typeof claims.iss !== 'string' || !ALLOWED_ISSUERS.has(claims.iss)) { + return false; + } return true; } diff --git a/tests/auth-context.test.js b/tests/auth-context.test.js index a056510..73b22f2 100644 --- a/tests/auth-context.test.js +++ b/tests/auth-context.test.js @@ -58,9 +58,21 @@ const { useAuth, handleAuthResult, decodeIdToken, + isSessionStillValid, STORAGE_KEY, } = require('../lib/auth-context'); +// Build a JWT with arbitrary claims for the negative-path probes below. +// Signature is never verified by this client — only the payload is decoded. +function makeFakeJwt(claimsOverride) { + const payload = Buffer.from(JSON.stringify(claimsOverride)) + .toString('base64') + .replace(/=+$/, '') + .replace(/\+/g, '-') + .replace(/\//g, '_'); + return `eyJhbGciOiJSUzI1NiJ9.${payload}.sig`; +} + function Probe({ onUser }) { const ctx = useAuth(); onUser(ctx); @@ -91,6 +103,58 @@ describe('decodeIdToken', () => { }); }); +describe('isSessionStillValid — issuer + expiry boundary', () => { + const future = Math.floor(Date.now() / 1000) + 3600; + const past = Math.floor(Date.now() / 1000) - 3600; + + test('accepts a fresh Google-issued token', () => { + const tok = makeFakeJwt({ + iss: 'https://accounts.google.com', + sub: 'goog-1', + exp: future, + }); + expect(isSessionStillValid({ tokens: { idToken: tok } })).toBe(true); + }); + + test('rejects a token whose iss is missing — regression for the second-pass audit', () => { + // Without this check, an attacker who can write to SecureStore (e.g. on a + // rooted/jailbroken device or via a shared-keychain entitlement bug) could + // craft an iss-less blob and ride the session indefinitely. See + // lib/auth-context.js — `iss` is REQUIRED, not optional. + const tok = makeFakeJwt({ sub: 'x', exp: future }); + expect(isSessionStillValid({ tokens: { idToken: tok } })).toBe(false); + }); + + test('rejects a token issued by a non-Google IdP', () => { + const tok = makeFakeJwt({ + iss: 'https://evil.example.com', + sub: 'x', + exp: future, + }); + expect(isSessionStillValid({ tokens: { idToken: tok } })).toBe(false); + }); + + test('rejects an expired token even with a valid iss', () => { + const tok = makeFakeJwt({ + iss: 'https://accounts.google.com', + sub: 'x', + exp: past, + }); + expect(isSessionStillValid({ tokens: { idToken: tok } })).toBe(false); + }); + + test('rejects when exp is missing entirely', () => { + const tok = makeFakeJwt({ iss: 'https://accounts.google.com', sub: 'x' }); + expect(isSessionStillValid({ tokens: { idToken: tok } })).toBe(false); + }); + + test('rejects when no idToken is present', () => { + expect(isSessionStillValid({ tokens: {} })).toBe(false); + expect(isSessionStillValid({})).toBe(false); + expect(isSessionStillValid(null)).toBe(false); + }); +}); + describe('handleAuthResult (pure)', () => { test('success result populates user + writes SecureStore', async () => { const setSession = jest.fn(); From 7d9a9a48e7f9d2206cee1a5f01421ff2ce1e43c4 Mon Sep 17 00:00:00 2001 From: heznpc Date: Thu, 21 May 2026 22:18:48 +0900 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20second-pass=20audit=20Major=20batch?= =?UTF-8?q?=20=E2=80=94=20close=20test/CI=20catch-rate=20gaps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layered on top of the iss-bypass fix in this same branch. All seven items were surfaced by the 2026-05-21 adversarial second-pass on a session that had reported "tests pass / CI green" — pure A3/A2/D1 catch-rate gaps in the prior audit. M1 + M5 — Version bumper had a trivially-passing test (file-existence check only) and crashed on prerelease versions: - scripts/bump-version.js: reject anything not strict MAJOR.MINOR.PATCH (previously `"1.2.3-beta.1".split('.').map(Number)` produced NaN and would have committed `1.2.NaN` to app.json) - tests/app.test.js: replace the existence check with 7 behavioural tests that exercise the bumper in a sandboxed tmpdir M2 — handleAuthResult silently persisted a `{user:null,tokens:{...:null}}` blob when the OAuth response was `type:'success'` with no id_token. Self-healing on next mount but a real state-corruption bug. - lib/auth-context.js: throw on missing id_token, do not persist - tests/auth-context.test.js: regression test M3 — (app)/_layout.js gating ("Currently implemented" in README) was never exercised by any test. screens.test.js mocked useAuth to always return a user, so the redirect/loading branches were 0% covered. - tests/layout.test.js: NEW file. Covers both (app) and (auth) layouts across loading / signed-out / signed-in states. The "D1 promised, not verified" gap on the auth boundary is now closed. M4 — assertGoogleEnv throw + signIn body were uncovered: - tests/env.test.js: NEW. Locks the throw branch contract. Caveat in the file header: Expo's babel preset inlines EXPO_PUBLIC_* at compile time, which restricts how we can probe the positive case. - tests/signin-error.test.js: NEW. Isolated mock of lib/env to cover signIn's catch → setError → rethrow path. M6 — ci.yml "Build verification" step swallowed errors via `2>/dev/null || echo "skipping"`. Removed. Web export is a non-goal per README, and the step's only effect was producing a false-green signal. M7 — ci.yml `npm audit --audit-level=critical` masked 23 production advisories. Raised to `--audit-level=high`. The fix path is the Expo SDK 52 → 55 upgrade tracked in the modernization audit; CI going red here is the intended forcing function until that upgrade lands. If a single transitive advisory turns out to be unfixable, document the exception inline rather than re-weakening the gate. Test results: - Before second-pass: 19/19, 80.2 / 62.5 / 75 / 85.88 - After (Critical + Major): 43/43, 93.57 / 78.04 / 86.36 / 96.96 CI may go red on the audit step until SDK 52 → 55 lands. Explicit user decision in the audit session: "둘 다 지금 — CI 빨개질 가능성을 의도된 경고로 받음". --- .github/workflows/ci.yml | 19 ++++++--- lib/auth-context.js | 10 +++++ scripts/bump-version.js | 16 ++++++- tests/app.test.js | 77 ++++++++++++++++++++++++++++++++- tests/auth-context.test.js | 61 +++++++++++++++++++++++++- tests/env.test.js | 45 ++++++++++++++++++++ tests/layout.test.js | 87 ++++++++++++++++++++++++++++++++++++++ tests/signin-error.test.js | 64 ++++++++++++++++++++++++++++ 8 files changed, 369 insertions(+), 10 deletions(-) create mode 100644 tests/env.test.js create mode 100644 tests/layout.test.js create mode 100644 tests/signin-error.test.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 02e67af..a68df34 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,9 +52,16 @@ jobs: run: npx license-checker --failOn "GPL-2.0;GPL-3.0;AGPL-3.0" - name: Security audit - # Expo's transitive deps (tar, jsdom) frequently trigger high-severity - # advisories that can't be fixed without breaking Expo upgrades. - run: npm audit --audit-level=critical + # Threshold raised from `critical` to `high` in the 2026-05-21 + # second-pass audit. The previous `critical`-only gate masked + # 20+ high-severity production advisories (undici, tar, node-forge, + # @xmldom/xmldom, postcss, picomatch, brace-expansion, fast-uri). + # The fix path for all of them is the Expo SDK 52 → 55 upgrade + # tracked in the modernization audit — failing CI here is the + # forcing function for that upgrade. If a single transitive + # advisory genuinely can't be patched, document the exception + # inline rather than weakening this gate again. + run: npm audit --audit-level=high - name: Lint run: npm run lint @@ -62,5 +69,7 @@ jobs: - name: Test run: npm test - - name: Build verification - run: npx expo export --platform web 2>/dev/null || echo "Web export not configured — skipping" + # Web export is a non-goal for the starter (README "Non-Goals"). The + # previous `2>/dev/null || echo "skipping"` swallowed real build + # failures, so the step was producing a false-green signal. Removed + # entirely — `npm test` is the build verification for this project. diff --git a/lib/auth-context.js b/lib/auth-context.js index 2a128b8..cc81aff 100644 --- a/lib/auth-context.js +++ b/lib/auth-context.js @@ -106,6 +106,16 @@ export async function handleAuthResult(response, setSession, deps = {}) { response.params?.id_token ?? response.authentication?.idToken ?? null; const accessToken = response.authentication?.accessToken ?? response.params?.access_token ?? null; + // A `type:'success'` response without an id_token cannot establish + // identity — the scope requested includes `openid` so Google always + // returns one. Treating this as success would persist a `{user:null,...}` + // blob to SecureStore that's then deleted on the next mount: a real but + // self-healing state-corruption bug surfaced by the 2026-05-21 audit. + if (!idToken) { + throw new Error( + 'Auth success response missing id_token. Refusing to persist a userless session.', + ); + } const claims = decodeIdToken(idToken); const user = userFromClaims(claims); const session = { user, tokens: { idToken, accessToken } }; diff --git a/scripts/bump-version.js b/scripts/bump-version.js index 0302317..89a788a 100644 --- a/scripts/bump-version.js +++ b/scripts/bump-version.js @@ -8,7 +8,21 @@ if (!valid.includes(type)) { } const appConfig = JSON.parse(fs.readFileSync('app.json', 'utf8')); -const v = appConfig.expo.version.split('.').map(Number); +const current = appConfig.expo.version; + +// Only accept strict MAJOR.MINOR.PATCH numerics. Prerelease/build-metadata +// (`1.2.3-beta.1`, `1.2.3+sha`) would parse to NaN under the naive split-Map, +// producing a corrupted `1.2.NaN` write — fail loudly instead. +if (!/^\d+\.\d+\.\d+$/.test(current)) { + console.error( + `Refusing to bump: app.json expo.version="${current}" is not strict ` + + `MAJOR.MINOR.PATCH. Prerelease/build-metadata is not supported here — ` + + `bump manually and commit, or strip suffixes first.`, + ); + process.exit(1); +} + +const v = current.split('.').map(Number); if (type === 'major') { v[0]++; v[1] = 0; v[2] = 0; } else if (type === 'minor') { v[1]++; v[2] = 0; } diff --git a/tests/app.test.js b/tests/app.test.js index c9fe22c..f83b9ce 100644 --- a/tests/app.test.js +++ b/tests/app.test.js @@ -74,8 +74,81 @@ describe('Project structure', () => { describe('Version bumper', () => { const bumperPath = path.resolve(__dirname, '..', 'scripts', 'bump-version.js'); + const { execFileSync } = require('child_process'); + const os = require('os'); - test('bump script exists', () => { - expect(fs.existsSync(bumperPath)).toBe(true); + // Run the bumper against a throwaway sandbox so we don't mutate the real + // app.json. The script reads/writes from process.cwd(), so we cd into a + // tmpdir seeded with a minimal app.json (and optionally a package.json). + function runBumper({ type, appVersion, withPackageJson = false }) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'bump-test-')); + fs.writeFileSync( + path.join(dir, 'app.json'), + JSON.stringify({ expo: { version: appVersion } }) + '\n', + ); + if (withPackageJson) { + fs.writeFileSync( + path.join(dir, 'package.json'), + JSON.stringify({ name: 'sandbox', version: appVersion }) + '\n', + ); + } + let err = null; + let stdout = ''; + try { + stdout = execFileSync(process.execPath, [bumperPath, type], { + cwd: dir, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + } catch (e) { + err = e; + } + const finalApp = JSON.parse(fs.readFileSync(path.join(dir, 'app.json'), 'utf8')); + const finalPkg = withPackageJson + ? JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf8')) + : null; + return { stdout: stdout.trim(), err, app: finalApp, pkg: finalPkg }; + } + + test('patch bump increments only the patch component', () => { + const r = runBumper({ type: 'patch', appVersion: '1.2.3' }); + expect(r.err).toBeNull(); + expect(r.app.expo.version).toBe('1.2.4'); + expect(r.stdout).toBe('1.2.4'); + }); + + test('minor bump zeroes the patch component', () => { + const r = runBumper({ type: 'minor', appVersion: '1.2.3' }); + expect(r.app.expo.version).toBe('1.3.0'); + }); + + test('major bump zeroes minor and patch', () => { + const r = runBumper({ type: 'major', appVersion: '1.2.3' }); + expect(r.app.expo.version).toBe('2.0.0'); + }); + + test('mirrors the new version into package.json when present', () => { + const r = runBumper({ type: 'patch', appVersion: '1.2.3', withPackageJson: true }); + expect(r.pkg.version).toBe('1.2.4'); + }); + + test('rejects prerelease versions instead of writing NaN — regression', () => { + // Before the second-pass audit (2026-05-21), `"1.2.3-beta.1".split('.')` + // -> `Number('3-beta')` = NaN, producing `1.2.NaN` in app.json. + const r = runBumper({ type: 'patch', appVersion: '1.2.3-beta.1' }); + expect(r.err).not.toBeNull(); + expect(r.app.expo.version).toBe('1.2.3-beta.1'); // unchanged + }); + + test('rejects 2-component versions instead of writing NaN — regression', () => { + const r = runBumper({ type: 'patch', appVersion: '1.2' }); + expect(r.err).not.toBeNull(); + expect(r.app.expo.version).toBe('1.2'); // unchanged + }); + + test('rejects invalid bump type', () => { + const r = runBumper({ type: 'wat', appVersion: '1.2.3' }); + expect(r.err).not.toBeNull(); + expect(r.app.expo.version).toBe('1.2.3'); // unchanged }); }); diff --git a/tests/auth-context.test.js b/tests/auth-context.test.js index 73b22f2..14c2cf8 100644 --- a/tests/auth-context.test.js +++ b/tests/auth-context.test.js @@ -30,8 +30,20 @@ jest.mock('expo-auth-session/providers/google', () => ({ useAuthRequest: () => [{ state: 'ready' }, null, jest.fn(async () => ({ type: 'cancel' }))], })); -// Env: pretend the web client ID is set. -process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID = 'test-web-client-id'; +// Stub lib/env so assertGoogleEnv is a no-op for the happy-path suite. The +// error path (assertGoogleEnv throws) lives in tests/signin-error.test.js +// where the same module is mocked the other way. The previous +// `process.env.EXPO_PUBLIC_... = ...` approach worked while no test invoked +// signIn(), but env.js reads process.env at module-load time — the load +// order vs. the assignment is fragile under jest-expo's preset hoisting. +jest.mock('../lib/env', () => ({ + googleClientIds: { + webClientId: 'test-web-client-id', + iosClientId: undefined, + androidClientId: undefined, + }, + assertGoogleEnv: jest.fn(), +})); // --- Fixture: a Google id_token (not signature-verified, only decoded) --- // header.payload.signature where payload is base64url(JSON) @@ -197,6 +209,34 @@ describe('handleAuthResult (pure)', () => { expect(setSession).not.toHaveBeenCalled(); }); + test('success without id_token throws and does NOT persist — regression', async () => { + // Before the second-pass audit (2026-05-21), this path silently wrote + // `{user:null,tokens:{idToken:null,accessToken:null}}` to SecureStore. + // Self-healing on next mount, but a real state-corruption bug. + const setSession = jest.fn(); + const store = { + setItemAsync: jest.fn(), + getItemAsync: jest.fn(), + deleteItemAsync: jest.fn(), + }; + await expect( + handleAuthResult( + { type: 'success', params: {}, authentication: {} }, + setSession, + { store }, + ), + ).rejects.toThrow(/missing id_token/); + expect(store.setItemAsync).not.toHaveBeenCalled(); + expect(setSession).not.toHaveBeenCalled(); + }); + + test('unknown response type (e.g. dismiss) is a no-op', async () => { + const setSession = jest.fn(); + await handleAuthResult({ type: 'dismiss' }, setSession); + expect(setSession).not.toHaveBeenCalled(); + expect(mockSecureStore.setItemAsync).not.toHaveBeenCalled(); + }); + // Mutation-check: if handleAuthResult accidentally skipped persisting the // session, the assertion below would still pass for user population but this // extra check fails. Guards against a common regression. @@ -248,6 +288,23 @@ describe('AuthProvider lifecycle', () => { expect(captured.user?.email).toBe('restored@example.com'); }); + test('signIn (happy path) calls promptAsync without throwing', async () => { + // Covers lines 165-173 + 178 of lib/auth-context.js (signIn body, no-throw + // branch). The error path is covered in tests/signin-error.test.js with + // isolated mocking — see that file for the assertGoogleEnv-throws scenario. + let captured; + render( + + (captured = c)} /> + , + ); + await waitFor(() => expect(captured.loading).toBe(false)); + await act(async () => { + await captured.signIn(); + }); + expect(captured.error).toBeNull(); + }); + test('signOut clears user + SecureStore', async () => { mockMemStore.set( STORAGE_KEY, diff --git a/tests/env.test.js b/tests/env.test.js new file mode 100644 index 0000000..d74b3d5 --- /dev/null +++ b/tests/env.test.js @@ -0,0 +1,45 @@ +// Lock the env assertion contract that lib/env.js promises in its docstring. +// Previously uncovered (env.js:23-24 throw branch) — added in the 2026-05-21 +// second-pass audit. +// +// Caveat: Expo's babel preset enables +// `babel-plugin-transform-inline-environment-variables` for EXPO_PUBLIC_* +// vars, which inlines `process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID` at +// compile time. That means re-importing env.js after mutating process.env +// at runtime is a no-op — the compiled module captured whatever the env +// was when this test file was first transformed. +// +// Practical consequence: the positive-path ("does not throw when set", +// "exposes IDs") is covered indirectly by tests/auth-context.test.js (which +// mocks lib/env). The two tests below isolate just the throw branch by +// running env.js in an unset state — which is what happens when CI runs +// without dev secrets configured. + +describe('assertGoogleEnv (throw branch only — see header comment)', () => { + const KEY = 'EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID'; + let saved; + + beforeEach(() => { + saved = process.env[KEY]; + delete process.env[KEY]; + }); + + afterEach(() => { + if (saved === undefined) delete process.env[KEY]; + else process.env[KEY] = saved; + }); + + test('throws a helpful error when the web client ID is missing', () => { + jest.isolateModules(() => { + const { assertGoogleEnv } = require('../lib/env'); + expect(() => assertGoogleEnv()).toThrow(/EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID/); + }); + }); + + test('error message points the user at .env.example and the README', () => { + jest.isolateModules(() => { + const { assertGoogleEnv } = require('../lib/env'); + expect(() => assertGoogleEnv()).toThrow(/\.env\.example|README/); + }); + }); +}); diff --git a/tests/layout.test.js b/tests/layout.test.js new file mode 100644 index 0000000..927a666 --- /dev/null +++ b/tests/layout.test.js @@ -0,0 +1,87 @@ +// Exercise the route-group gating that README "Currently implemented" lists +// as the auth boundary. Before the 2026-05-21 second-pass audit, no test +// rendered (app)/_layout.js — the gating was claimed, not verified. + +import React from 'react'; +import { render } from '@testing-library/react-native'; +import { Text } from 'react-native'; + +// Capture what `` was rendered with so we can assert the gate. +const mockRedirect = jest.fn(({ href }) => null); +const mockStack = jest.fn(() => null); + +jest.mock('expo-router', () => ({ + Redirect: (props) => mockRedirect(props), + Stack: (props) => mockStack(props), +})); + +// Mock useAuth per-test by re-requiring after redefining the mock factory. +let mockAuthState; +jest.mock('../lib/auth-context', () => ({ + useAuth: () => mockAuthState, +})); + +describe('(app)/_layout — protected route group', () => { + beforeEach(() => { + mockRedirect.mockClear(); + mockStack.mockClear(); + }); + + test('while loading, shows the spinner (no redirect, no Stack)', () => { + mockAuthState = { user: null, loading: true }; + const AppLayout = require('../app/(app)/_layout').default; + const { UNSAFE_root } = render(); + expect(mockRedirect).not.toHaveBeenCalled(); + expect(mockStack).not.toHaveBeenCalled(); + // The spinner View is rendered — assert by tree shape rather than text. + expect(UNSAFE_root).toBeTruthy(); + }); + + test('when not signed in, redirects to /login (regression for D1 gating claim)', () => { + mockAuthState = { user: null, loading: false }; + const AppLayout = require('../app/(app)/_layout').default; + render(); + expect(mockRedirect).toHaveBeenCalledWith({ href: '/login' }); + expect(mockStack).not.toHaveBeenCalled(); + }); + + test('when signed in, renders the Stack (protected zone is accessible)', () => { + mockAuthState = { user: { email: 'a@b.c' }, loading: false }; + const AppLayout = require('../app/(app)/_layout').default; + render(); + expect(mockStack).toHaveBeenCalled(); + expect(mockRedirect).not.toHaveBeenCalled(); + }); +}); + +describe('(auth)/_layout — bounce when already signed in', () => { + beforeEach(() => { + mockRedirect.mockClear(); + mockStack.mockClear(); + }); + + test('while loading, neither Redirect nor a bounce fires', () => { + mockAuthState = { user: null, loading: true }; + const AuthLayout = require('../app/(auth)/_layout').default; + render(); + expect(mockRedirect).not.toHaveBeenCalled(); + // Stack still rendered while loading — that's intentional, the parent + // (app)/_layout owns the spinner. + expect(mockStack).toHaveBeenCalled(); + }); + + test('when signed in, bounces back to /', () => { + mockAuthState = { user: { email: 'a@b.c' }, loading: false }; + const AuthLayout = require('../app/(auth)/_layout').default; + render(); + expect(mockRedirect).toHaveBeenCalledWith({ href: '/' }); + }); + + test('when not signed in, renders the Stack so the login screen appears', () => { + mockAuthState = { user: null, loading: false }; + const AuthLayout = require('../app/(auth)/_layout').default; + render(); + expect(mockRedirect).not.toHaveBeenCalled(); + expect(mockStack).toHaveBeenCalled(); + }); +}); diff --git a/tests/signin-error.test.js b/tests/signin-error.test.js new file mode 100644 index 0000000..1a5d2dc --- /dev/null +++ b/tests/signin-error.test.js @@ -0,0 +1,64 @@ +// Cover the signIn error path (auth-context.js:174-177) — previously +// uncovered. The path: assertGoogleEnv() throws → catch → setError(e) → rethrow. +// Isolated in its own file so we can mock lib/env without disturbing the +// happy-path fixture in tests/auth-context.test.js. + +import React from 'react'; +import { act, render, waitFor } from '@testing-library/react-native'; +import { Text } from 'react-native'; + +const mockMemStore = new Map(); +const mockSecureStore = { + getItemAsync: jest.fn(async (k) => (mockMemStore.has(k) ? mockMemStore.get(k) : null)), + setItemAsync: jest.fn(async (k, v) => { + mockMemStore.set(k, v); + }), + deleteItemAsync: jest.fn(async (k) => { + mockMemStore.delete(k); + }), +}; + +jest.mock('expo-secure-store', () => mockSecureStore); +jest.mock('expo-web-browser', () => ({ maybeCompleteAuthSession: jest.fn() })); +jest.mock('expo-auth-session/providers/google', () => ({ + useAuthRequest: () => [{ state: 'ready' }, null, jest.fn()], +})); + +// Replace lib/env with a stub whose assertGoogleEnv always throws — this +// simulates the "developer forgot to set EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID" +// situation at the boundary of signIn() rather than at module load. +jest.mock('../lib/env', () => ({ + googleClientIds: { webClientId: undefined, iosClientId: undefined, androidClientId: undefined }, + assertGoogleEnv: () => { + throw new Error('Missing EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID. (test stub)'); + }, +})); + +const { AuthProvider, useAuth } = require('../lib/auth-context'); + +function Probe({ onUser }) { + const ctx = useAuth(); + onUser(ctx); + return {ctx.user ? 'user' : 'anon'}; +} + +describe('signIn error path — env missing', () => { + test('rethrows from signIn AND lights up context.error', async () => { + let captured; + render( + + (captured = c)} /> + , + ); + await waitFor(() => expect(captured.loading).toBe(false)); + + await expect( + act(async () => { + await captured.signIn(); + }), + ).rejects.toThrow(/Missing EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID/); + + await waitFor(() => expect(captured.error).toBeInstanceOf(Error)); + expect(captured.error.message).toMatch(/Missing EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID/); + }); +}); From 328968111d452ca15c9a1b0ec73e68affbdf278d Mon Sep 17 00:00:00 2001 From: heznpc Date: Thu, 21 May 2026 22:20:06 +0900 Subject: [PATCH 3/3] chore(audit): inline TODO for corrupt-blob purge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Minor finding from the 2026-05-21 second-pass: lib/auth-context.js:165 swallows a SecureStore JSON.parse failure without deleting the blob. Self-healing on next successful sign-in (setItemAsync overwrites), but the explicit-purge is cheap and the silent-swallow is the kind of thing that hides the next bug. Filed as inline TODO per audit policy — no separate PR. --- lib/auth-context.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/auth-context.js b/lib/auth-context.js index cc81aff..7a03b8f 100644 --- a/lib/auth-context.js +++ b/lib/auth-context.js @@ -164,6 +164,9 @@ export function AuthProvider({ children }) { } } catch { // corrupt blob -> treat as signed out + // TODO(2nd-pass-audit-2026-05-21): also call deleteItemAsync here + // so a corrupt blob is purged rather than re-read on every mount. + // Self-healing on next successful sign-in, but explicit is better. } finally { if (!cancelled) setLoading(false); }