fix(db): reconcile hosted privacy recovery state - #2464
Conversation
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4200df55ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const supabase = await createClient(); | ||
| const { data: { user }, error } = await supabase.auth.getUser(); | ||
| userId = !error && typeof user?.id === 'string' && UUID_PATTERN.test(user.id) ? user.id : null; |
There was a problem hiding this comment.
recoverExistingPasswordAccount accepts any valid Supabase session from getUser() and immediately records the confirmation as password_signup; it never verifies that the session was created by the password attempt performed by the UI. An ineligible OAuth session retained on /privacy/onboarding, or any older session cookie, can therefore create a password-intent challenge and POST {"action":"existing_account"} directly to bypass the intended password reauthentication and OAuth nonce path.
Useful? React with 👍 / 👎.
| IF COALESCE((v_challenge.requested_consents ->> 'email')::boolean, false) THEN | ||
| INSERT INTO privacy_retention.privacy_consent_events (user_id, subject_kind, purpose, channel, decision, policy_version_id, notice_sha256, source, correlation_id, idempotency_key) | ||
| VALUES (p_user_id, 'self', 'email_marketing', 'email', 'granted', v_policy.id, v_policy.content_sha256, p_source, p_challenge_id, v_idempotency_prefix || 'email'); |
There was a problem hiding this comment.
When an existing account already has a granted marketing event for the current published policy and reattests with this channel unchecked, this branch inserts nothing, so privacy_consent_state continues to expose the previous grant as the latest decision; the same problem applies to SMS, push, and each night channel below. Reattestation must append an explicit denied event for every false choice so advertising cannot continue after the user declines it.
AGENTS.md reference: AGENTS.md:L50-L50
Useful? React with 👍 / 👎.
| GRANT EXECUTE ON FUNCTION privacy_retention.assert_g014_catalog_contract() | ||
| TO postgres; |
There was a problem hiding this comment.
This changes the contents of the already-applied 20260713002500 migration, while the same commit also edits 20260713002000. Hosted databases have already executed the previous bytes, so fresh reconstruction now grants/revokes a different catalog than production and the retained migration evidence no longer identifies one immutable history; restore both files and put these corrections in a new migration.
AGENTS.md reference: AGENTS.md:L54-L54
Useful? React with 👍 / 👎.
| const challenge = readOnboardingChallenge(challengeCookie); | ||
| if (challengeCookie && !challenge) return rejectedCallbackRedirect(request, origin); | ||
| const onboardingRequested = challenge?.intent === 'oauth'; |
There was a problem hiding this comment.
If Google signup creates the challenge but OAuth startup fails or the user abandons the provider flow, the sealed challenge cookie remains until expiry. A subsequent click on the ordinary Google Login button produces a normal callback with no onboarding marker, yet this line classifies it as onboarding solely because of that stale cookie and can apply the abandoned challenge's age and marketing selections; bind intent to the specific redirect attempt or clear the challenge when startup is abandoned.
Useful? React with 👍 / 👎.
| || pathname === '/privacy' | ||
| || pathname === '/data-deletion' |
There was a problem hiding this comment.
Classifying /privacy and /data-deletion as credentialless-public means they stop bypassing session enforcement whenever the request carries a Supabase cookie hint. A visitor with an expired, malformed, or privacy-ineligible session is then signed out and redirected to /auth/required instead of being allowed to read these public legal pages; these two paths were explicitly loop-safe before this change and should remain accessible regardless of session state.
Useful? React with 👍 / 👎.
| if (isExistingAccountRecovery) recoveryToken = beginExistingAccountPrivacyRecovery(email); | ||
| const { data: existingSession, error: existingSessionError } = await supabase.auth.signInWithPassword({ email, password }); |
There was a problem hiding this comment.
The new existing-account recovery path runs only after handleSignup has rejected every password longer than 12 characters. Existing Supabase accounts can legitimately have longer passwords—the onboarding API itself accepts up to 72 characters—so affected users can log in far enough to be told to reattest but can never reach this recovery attempt without first shortening their password; apply the 12-character signup restriction only when creating a new account.
Useful? React with 👍 / 👎.
| 'eligible', true, 'status', 'applied', | ||
| 'readback', pg_catalog.jsonb_build_object('passed', true, 'checks', pg_catalog.jsonb_build_object( | ||
| 'challengeConsumed', true, 'ageProfileRecorded', true, 'requiredConsentRecorded', true, 'eligible', true |
There was a problem hiding this comment.
These receipt fields are hard-coded rather than derived from canonical eligibility. For example, a user with an active privacy_onboarding_compensation_holds row can pass the profile checks, but that hold still makes g014_privacy_eligibility_receipt return PRIVACY_AGE_BLOCKED; this function nevertheless consumes the challenge and permanently appends an ONBOARDING_CONFIRMED audit claiming eligible: true, after which the API's separate readback returns failure. Reject the transition or query the canonical eligibility result before emitting the applied audit and receipt.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
245afee to
f1018f2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1018f294b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function classifyEligibility(eligibility: CurrentPrivacyEligibility): RosterClassification { | ||
| if (eligibility.eligible === true && eligibility.reasonCode === 'PRIVACY_ELIGIBLE') { | ||
| return 'already_current_eligible'; |
There was a problem hiding this comment.
When getCurrentPrivacyEligibilityForUser returns an otherwise valid PRIVACY_ELIGIBLE receipt whose content SHA differs from the current policy SHA, the existing eligibility helper still sets the top-level fields used here to eligible: true and reasonCode: 'PRIVACY_ELIGIBLE'. This branch therefore durably records already_current_eligible without examining the receipt, and retries preserve the stale result; require hasLivePrivacyEligibilityReceipt or an equivalent exact policy binding before assigning this classification.
AGENTS.md reference: AGENTS.md:L63-L63
Useful? React with 👍 / 👎.
| export function emitPrivacyAuthEvent(input: unknown, now: Date = new Date()): EmittedPrivacyAuthEvent { | ||
| if (typeof window !== 'undefined') { | ||
| throw new Error('Privacy auth events can only be emitted on the server.'); | ||
| } | ||
|
|
||
| const event = validatePrivacyAuthEvent(input); | ||
| const emitted = { utcMinute: formatPrivacyAuthUtcMinute(now), ...event }; | ||
| console.info(JSON.stringify(emitted)); |
There was a problem hiding this comment.
In the reviewed e68efd1 tree, a repo-wide reference search finds this emitter only at its definition and in its unit test; onboarding, callback, middleware, logout, roster, and release runtime paths never call it. Consequently production canaries emit none of the allowlisted events required by the new runbook, so the Datadog failure-rate and blocking monitors remain silent even when those flows fail.
Useful? React with 👍 / 👎.
| 'audit_write_failed', | ||
| 'catalog_drift', | ||
| 'roster_conservation_mismatch', | ||
| 'release_verified', |
There was a problem hiding this comment.
The runbook requires queries and injections for privacy-workflow 42501, eligibility errors, and policy drift, but none of those signals can be represented by this closed enum, and validatePrivacyAuthEvent rejects any unlisted outcome. Even after runtime call sites are added, the required privacy-workflow-42501-v1 monitor and eligibility/policy-drift release checks therefore cannot distinguish their target failures from generic failed events.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,47 @@ | |||
| { | |||
| "schemaVersion": 1, | |||
| "recordedAt": "2026-08-02T15:04:00Z", | |||
There was a problem hiding this comment.
This receipt says it was recorded at 2026-08-02T15:04:00Z, while its own createdAtUnixMs converts to 2026-08-02T18:09:13.488Z. A receipt recorded more than three hours before the deployment existed cannot validly attest that the deployment was already READY or that its production readbacks succeeded, so operators must not use this artifact as the pinned rollback evidence.
AGENTS.md reference: AGENTS.md:L74-L74
Useful? React with 👍 / 👎.
| return { | ||
| batchDigest: digest(batchId), | ||
| counts, | ||
| subjects, |
There was a problem hiding this comment.
If an operator retries a batch ID with a changed set of 16 users, the sink reuses results for overlapping users, inserts results for new users, and this function still returns the same batchDigest because it hashes only batchId. The resulting receipt is indistinguishable from the original batch despite representing a different roster, defeating idempotent readback; persist or hash the normalized manifest with the batch identity and reject a mismatch.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
| }, | ||
| "compatibility": { | ||
| "admissionModel": "current-policy schema-v1 live eligibility receipt", | ||
| "migrationVersion": "20260801000100", |
There was a problem hiding this comment.
The fallback advertises migration 20260801000100, but the pinned web source calls confirm_privacy_onboarding with the sixth p_oauth_nonce_hash argument. That signature is not created until 20260801000200, and its service-role allowlist is corrected in 20260801000300; against a database at the recorded migration version, every password or OAuth onboarding confirmation returns an RPC error. Record and verify the exact 00300 terminal state instead of treating 00100 as compatible.
AGENTS.md reference: AGENTS.md:L65-L65
Useful? React with 👍 / 👎.
| export async function classifyPrivacyRoster( | ||
| batchId: string, | ||
| userIds: readonly string[], | ||
| dependencies: RosterClassificationDependencies, | ||
| ): Promise<RosterClassificationResult> { |
There was a problem hiding this comment.
In the reviewed tree, classifyPrivacyRoster is referenced only by its unit test, and there is no implementation of RosterClassificationSink backed by Supabase or another durable store. The runbook nevertheless requires immutable roster proof before promotion, so operators currently have no executable path that reads the service eligibility RPC, persists the 16 classifications, or produces that proof; add a bounded server-side operation or backend command with a real sink and readback.
Useful? React with 👍 / 👎.
| function isStoredResult(value: StoredRosterClassification, batchId: string, userId: string) { | ||
| return value.batchId === batchId | ||
| && value.userId === userId | ||
| && CLASSIFICATIONS.has(value.classification) | ||
| && SHA256_PATTERN.test(value.subjectDigest) | ||
| && SHA256_PATTERN.test(value.receiptDigest) | ||
| && SHA256_PATTERN.test(value.resultDigest); |
There was a problem hiding this comment.
isStoredResult accepts any classification and any three syntactically valid SHA strings without checking that subjectDigest === digest(userId) or that resultDigest matches the stored batch, user, classification, and receipt digest. A sink serialization error or modified durable row can therefore change a subject from held to already_current_eligible while retaining arbitrary 64-character digests, and replay silently incorporates the corrupted count; recompute and compare both derived digests before accepting readback.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
| const response = await page.goto('/auth/callback?code=first&code=second'); | ||
| expect(response?.status()).toBe(200); | ||
| await expect(page).toHaveURL(/\/$/); | ||
| expect(providerRequests).toBe(0); |
There was a problem hiding this comment.
page.route intercepts requests issued by the browser page, but the Supabase exchangeCodeForSession call for /auth/callback runs inside the Next.js route handler. providerRequests therefore remains zero even if the server exchanges both ambiguous codes, so this assertion cannot verify the behavior named by the test and a pre-validation regression would pass; mock or instrument the server Supabase client and assert its exchange call count instead.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
| test('incomplete sessions keep the literal loop-safe onboarding route, but protected admin and API surfaces deny access', async ({ page, request }) => { | ||
| await page.goto('/privacy/onboarding'); | ||
| await expect(page).toHaveURL(/\/privacy\/onboarding$/); | ||
|
|
||
| const apiResponse = await request.get('/api/privacy/consents'); |
There was a problem hiding this comment.
This test never creates an authenticated-but-ineligible session: the onboarding page is opened anonymously, and the request fixture calls the API and admin routes without an eligibility-bearing session cookie. The observed failures therefore prove only ordinary anonymous denial, so middleware could start admitting incomplete sessions to protected routes while this test still passes; establish an incomplete session in the tested context before checking loop-safe and protected behavior.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7363b98471
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'workflow_42501', | ||
| 'audit_write_failed', |
There was a problem hiding this comment.
Fresh evidence after the earlier no-wiring report is that this revision adds emitters for onboarding, callbacks, and middleware, but a tree-wide search still finds workflow_42501, audit_write_failed, catalog_drift, and roster_conservation_mismatch only in this enum and its unit test. None of the actual workflow, audit, catalog, or roster failure paths emits them, so the four page/block monitors mandated by privacy-auth-recovery-runbook.md lines 53-56 remain permanently silent.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
|
|
||
| export async function GET(request: Request) { | ||
| const { searchParams, origin } = new URL(request.url); | ||
| emitCallbackPrivacyAuthEvent('callback_started'); |
There was a problem hiding this comment.
The fresh callback wiring emits only callback_started, before query validation, and no later branch emits failed, onboarding_required, or admitted. Consequently malformed requests and provider/exchange/eligibility failures all contribute starts but no failures, so the callback failure-rate monitor required by privacy-auth-recovery-runbook.md line 58 always reports zero failures and can be diluted further by arbitrary public callback requests.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
| emitMiddlewarePrivacyAuthEvent( | ||
| request, | ||
| eligibility.reasonCode === 'PRIVACY_POLICY_UNAVAILABLE' ? 'policy_drift' : 'denied', | ||
| ); |
There was a problem hiding this comment.
When getCurrentPrivacyEligibility encounters an RPC error, exception, or malformed receipt, it returns { reasonCode: null, receipt: null }; this conditional therefore records denied rather than eligibility_error. During an eligibility-service outage users still fail closed, but the runbook's eligibility-error rate and release-hold signal remain at zero, hiding precisely the failure they are intended to detect.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
| expect(challenge).not.toBeNull(); | ||
|
|
||
| const { GET } = await import(`@/app/auth/callback/route?state-machine=${Date.now()}`); | ||
| const response = await GET(new Request('http://localhost:3000/auth/callback?code=provider-code', { |
There was a problem hiding this comment.
This request omits the required flow digest, so the callback sets onboardingRequested to false and executes the ordinary-login branch despite the OAuth onboarding challenge cookie. The assertions therefore keep passing if the onboarding branch stops rejecting malformed user IDs before confirm_privacy_onboarding; include flow=sha256(oauthNonce) so the test actually verifies the mutation-order guarantee named in its title.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10cc5c8e08
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| const params = new URLSearchParams({ intent: "signup" }); | ||
| if (isAdminRedirect) params.set("next", safeRedirectTo); | ||
| window.location.assign(`/api/auth/oauth?${params.toString()}`); |
There was a problem hiding this comment.
When an ordinary Google callback finds an ineligible account, it retains that session and redirects to /privacy/onboarding; clicking Google signup then reaches this navigation with the ineligible cookie still present. Neither GET /api/auth/oauth nor POST /api/auth/password-login is included in classifyPublicEligibilitySessionRoute's loop-safe allowlist, so the proxy runs eligibility enforcement and returns 403 before either new handler can execute, blocking reattestation. Add the exact path/method pairs and corresponding classifier assertions.
AGENTS.md reference: AGENTS.md:L78-L78
Useful? React with 👍 / 👎.
| if (!response.ok || outcome !== "admitted") throw new Error("password_login_failed"); | ||
|
|
||
| toast.success("로그인 성공!"); | ||
| dispatchHomeAuthSessionUpdated({ |
There was a problem hiding this comment.
On non-home pages where AuthProvider has already bootstrapped an anonymous session, the server-side password endpoint updates cookies without triggering the browser Supabase client's onAuthStateChange. This custom event is consumed by home-runtime-shell.tsx and auth-UI helpers, but not by AuthContext, so the modal closes while useAuth() continues exposing a logged-out user until a reload or navigation. Explicitly make the mounted provider reload/publish the new session after an admitted response.
Useful? React with 👍 / 👎.
| }); | ||
| if (!transaction) return rejectedResponse(url.origin); | ||
|
|
||
| const callback = new URL('/auth/callback', trustedOrigin(url.origin)); |
There was a problem hiding this comment.
When a user starts Google auth on a production alias whose origin differs from NEXT_PUBLIC_SITE_URL or the default—for example tzudong.app while the canonical origin is www.tzudong.app—this constructs the callback on the other host. The signed transaction and onboarding challenge cookies are host-only, so the callback receives neither and is rejected before code exchange; the release configuration recognizes both production aliases and there is no hostname redirect in next.config.mjs, making Google login unusable on one alias. Use the initiating origin for the callback or canonicalize the browser before creating the cookies.
Useful? React with 👍 / 👎.
|
Terminal release review remains BLOCKED at Verified local gates: 93 focused unit tests, TypeScript, lint, 8 Chromium Playwright cases, and production build passed. Receipt-only admission, provider-only G016 tooling, server-bound OAuth initiation, keyed roster classification integrity, and correlated auth failure signals are implemented. Source-merge blockers still open: no production roster job/durable sink/exact 16-subject operations manifest; incomplete release/catalog and confirmation/audit telemetry producers; callback-rate semantics and behavioral transaction coverage require reconciliation; the added bounded Production promotion is independently blocked by missing genuine provider/ledger/catalog, policy/legal/operator, pinned fallback, Vercel/Datadog, controlled password/Google canary, roster batch, and 60-minute stabilization receipts. No G016 replay or privacy/consent evidence was manufactured. Terminal architect, red-team, and critic reviews all returned BLOCK. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b011d51d4e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const eligibility = await getCurrentPrivacyEligibility(supabase); | ||
| if (!hasLivePrivacyEligibilityReceipt(eligibility)) { | ||
| emitCallbackPrivacyAuthEvent('onboarding_required', correlationId); | ||
| return redirectWithOnboardingCookiesCleared(origin, '/privacy/onboarding'); |
There was a problem hiding this comment.
When get_current_privacy_eligibility errors, throws, or returns a malformed receipt, getCurrentPrivacyEligibility produces { reasonCode: null, receipt: null }, but this branch records onboarding_required and redirects the user into reattestation. The same unconditional classification occurs in api/auth/password-login/route.ts; during an eligibility-service outage both login paths therefore misdirect valid users and emit no eligibility_error, leaving the runbook's eligibility-error release signal at zero. Distinguish unavailable/invalid eligibility reads from genuine reattestation reason codes before emitting the terminal outcome.
Useful? React with 👍 / 👎.
| if (error || !data.url) { | ||
| emitOAuthCallbackEvent('failed', correlationId); | ||
| return rejectedResponse(url.origin); |
There was a problem hiding this comment.
When Supabase rejects OAuth initiation or returns no provider URL—for example during a provider outage or configuration error—this redirects to / without any fixed error marker or UI state. Because the modal has already navigated away, the user sees an unexplained page reload and, during signup, loses the consent selections they just entered; the previous client-side flow displayed a bounded Google-login/signup failure message. Redirect to a loop-safe error state that renders a generic failure, or otherwise preserve a fixed outcome the UI can consume.
Useful? React with 👍 / 👎.
| const normalizedUserIds = validateManifest(batchId, userIds); | ||
| if (!(dependencies.subjectPseudonymKey instanceof Uint8Array) || dependencies.subjectPseudonymKey.byteLength === 0) { | ||
| throw new Error('A non-empty server-held subject pseudonym key is required.'); |
There was a problem hiding this comment.
If an operational caller supplies a short key such as a one-byte Uint8Array, this validation accepts it and uses it for every subject HMAC and result signature. Anyone who obtains one known user-to-digest pair can then brute-force the key and link every pseudonym in the durable roster output, while also being able to reproduce its result signatures; requiring an appropriately sized server-held secret (for example at least 32 random bytes) prevents this configuration from silently defeating the opaque-subject boundary.
Useful? React with 👍 / 👎.
|
Follow-up hardening pushed in ee1741f: roster subject pseudonym keys now require at least 32 bytes, with focused rejection coverage. Verified |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee1741fddc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const SERVER_METADATA_ENVIRONMENT_KEYS = [ | ||
| 'VERCEL_GIT_COMMIT_SHA', | ||
| 'VERCEL_DEPLOYMENT_ID', | ||
| 'RELEASE_MIGRATION_MANIFEST_SHA256', |
There was a problem hiding this comment.
When the Vercel runtime does not explicitly define RELEASE_MIGRATION_MANIFEST_SHA256, every new runtime emitter is suppressed before logging. The only repository assignment is scoped to supabase-migration-apply.yml, so it is not inherited by the web deployment, and the new recovery runbook never instructs operators to provision or read back this required variable; following the runbook can therefore leave all six monitors silent. Add the runtime variable to the deployment configuration and verify its exact deployed value before relying on these events.
Useful? React with 👍 / 👎.
| || transaction.flow !== callback.flow | ||
| || transaction.next !== callback.next | ||
| ) { | ||
| emitCallbackPrivacyAuthEvent('failed', freshCorrelationId); | ||
| return rejectedCallbackRedirect(request, origin); |
There was a problem hiding this comment.
When OAuth is initiated in two tabs, the second attempt overwrites the singleton transaction cookie; if the first provider callback returns next, this mismatch branch rejects it and rejectedCallbackRedirect clears the second attempt's transaction and onboarding cookies as well. The second callback then also fails despite being the current valid attempt, so callbacks whose flow does not match the stored transaction should not delete that newer transaction, or the cookies should be keyed by flow.
Useful? React with 👍 / 👎.
|
CI is green at |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5867557e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| export async function POST(request: NextRequest) { | ||
| if (!isTrustedSameOriginMutation(request)) return loginResponse('auth_failed', 403); |
There was a problem hiding this comment.
When NEXT_PUBLIC_SITE_URL is https://www.tzudong.app, a login submitted from the supported https://tzudong.app alias carries that alias in its Origin header, so isTrustedSameOriginMutation rejects this new endpoint with 403 before checking credentials. The release configuration explicitly recognizes both production aliases and next.config.mjs does not canonicalize one to the other; because the modal now routes all password logins through this POST instead of the browser Supabase client, password login is unusable on the nonconfigured alias. Accept the exact trusted aliases or canonicalize navigation before submitting.
Useful? React with 👍 / 👎.
| const callback = new URL('/auth/callback', trustedOrigin(url.origin)); | ||
| callback.searchParams.set('next', next); | ||
| callback.searchParams.set('flow', flow); | ||
| emitOAuthCallbackEvent('callback_started', correlationId); |
There was a problem hiding this comment.
Fresh evidence after the earlier terminal-outcome report is that callback_started is now emitted by OAuth initiation, while the callback route emits failed even for requests with no valid transaction. A user who abandons Google produces a start with no terminal outcome, whereas any unsolicited malformed callback produces a failure with no start, so the callback failure-rate monitor required by privacy-auth-recovery-runbook.md lines 57-58 can be arbitrarily diluted or inflated rather than measuring callback executions. Emit the start when a callback request is accepted into the callback state machine and track initiation separately.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
| emitOnboardingPrivacyAuthEvent( | ||
| 'completed', | ||
| input.intent === 'oauth' ? 'oauth' : 'password', | ||
| correlationId, | ||
| ); |
There was a problem hiding this comment.
If PRIVACY_ONBOARDING_COOKIE_SECRET is absent or shorter than 32 bytes, the database challenge RPC can succeed and this emits completed, but sealOnboardingChallenge later returns null and the handler responds with 503 without emitting a failure. Canary telemetry therefore records a successful onboarding transition for a user-visible failure caused by a deployment misconfiguration; move this terminal event after the challenge has been sealed and attached to the successful response, and emit a bounded failure on the sealing branch.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| if (hasAuthCookie && (authFailed || !authUserId)) { | ||
| emitMiddlewarePrivacyAuthEvent(request, 'eligibility_error', eligibilityCorrelationId!); |
There was a problem hiding this comment.
When a browser presents an expired, revoked, or malformed Supabase auth cookie, getUser() fails before any eligibility RPC runs, yet this branch emits eligibility_error. These ordinary authentication failures therefore enter the runbook's eligibility-error numerator; with its threshold of more than 0.5% over 20 checks, even one stale session can falsely trigger a release hold. Record this as an authentication/session failure or exclude it from the eligibility-check denominator, reserving eligibility_error for failures of the eligibility read itself.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13d0a4939e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const total = Object.values(counts).reduce((sum, count) => sum + count, 0); | ||
| if (total !== ROSTER_CLASSIFICATION_SIZE || subjects.length !== ROSTER_CLASSIFICATION_SIZE) { | ||
| emitRosterClassificationEvent('roster_conservation_mismatch', correlationId); |
There was a problem hiding this comment.
Fresh evidence after the earlier no-wiring finding is that the new emitter is placed behind a tautological check: validation fixes the roster at 16 unique users, and every non-throwing loop iteration increments exactly one count and appends exactly one subject, so both totals must be 16 here. Real sink/readback failures throw earlier without emitting this outcome, leaving the required roster-conservation monitor silent; emit the alert from those actionable failure branches or validate an independent durable readback.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
| }; | ||
| const subjects: Array<ReturnType<typeof publicSubject>> = []; | ||
|
|
||
| for (const userId of normalizedUserIds) { |
There was a problem hiding this comment.
When the same 16-user batch is retried with its UUIDs in a different order, manifestDigest sorts the IDs and accepts the retry as the same manifest, but this loop builds subjects in caller-supplied order. The durable rows, counts, and batchDigest remain identical while the serialized result and any immutable artifact SHA change, so an order-only retry cannot produce stable idempotent readback; iterate the canonical sorted manifest or sort the public subjects before returning.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
| handlers.handleRequestEditRestaurant(restaurant); | ||
| }, | ||
| [handlers.handleRequestEditRestaurant], | ||
| [handlers], |
There was a problem hiding this comment.
useHomeHandlers returns a new wrapper object on every HomeClient render, so depending on handlers recreates this callback even when handlers.handleRequestEditRestaurant itself is unchanged. Because the callback is passed into memoized HomeMapContainer, unrelated parent updates now defeat that memo boundary and rerender the large map subtree; retain the member-level dependency used previously.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e79f328b08
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const result = getResultRecord(data); | ||
| if (error || !isExactChallengeReceipt(result, input, expiresAt)) { | ||
| emitOnboardingPrivacyAuthEvent( | ||
| error && isWorkflowAuthorizationError(error) ? 'workflow_42501' : 'audit_write_failed', |
There was a problem hiding this comment.
If the active policy changes after getCurrentPolicyVersion() but before the challenge RPC, create_privacy_onboarding_challenge raises 23514 (backend/supabase/migrations/20260713002100_g014_privacy_workflows.sql:466-467), but this ternary labels every non-42501 error—including that policy race and transport failures—as audit_write_failed. Because the runbook pages and blocks promotion on any audit-write-failure event, a normal policy cutover or RPC outage can produce a false audit incident; map the known policy error to policy_drift and reserve audit_write_failed for an explicitly identified audit failure.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e66450ab6b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isPublicRestrictedMode) return; | ||
| state.setIsReviewModalOpen(true); | ||
| }, []); | ||
| }, [state]); |
There was a problem hiding this comment.
useHomeState returns a memoized aggregate whose identity changes for virtually every home-state update, so depending on state recreates this callback even though setIsReviewModalOpen itself is stable. Since the callback is passed into memoized HomeMapContainer, unrelated changes such as edit-form or modal state now invalidate that memo boundary and rerender the large map subtree; retain a member-level dependency such as state.setIsReviewModalOpen instead.
Useful? React with 👍 / 👎.
| if (typeof batchId !== 'string' || batchId.trim().length === 0) { | ||
| throw new Error('A non-empty batchId is required.'); | ||
| } |
There was a problem hiding this comment.
If an operational caller uses an email address, free-form incident label, or another sensitive value as batchId, this validation accepts it and passes it unchanged to bindManifestIfAbsent and every durable classification row. The returned digest does not undo that raw persistence, so require a bounded opaque identifier format or generate the batch ID internally before crossing the sink boundary.
AGENTS.md reference: AGENTS.md:L51-L51
Useful? React with 👍 / 👎.
1 participant
Summary
Verification
Safety