fix(dashboard): symmetric RDE guard inside connectPaymentsSetup

Address PR feedback: connectPaymentsSetup lacked the RDE guard that
deferPaymentsSetup has, leaving asymmetrical defensive programming. The
Connect button is disabled in RDE at the JSX level, but the handler
itself unconditionally called setupPayments() + redirected to Stripe.
If the disabled attribute is ever bypassed — a future refactor dropping
the prop, DOM manipulation, programmatic invocation — the handler would
hit real Stripe.

Add an early-return at the top of connectPaymentsSetup mirroring the
existing guard pattern. Two layers of defense now:
1. JSX:   <DesignButton disabled={... || isRemoteDevelopmentEnvironment}>
2. Hook:  if (isRemoteDevelopmentEnvironment) return;

Add a test that proves the hook-level guard holds even when the visible
disabled attribute is removed mid-click (simulating exactly the bypass
scenario the reviewer described).
This commit is contained in:
Aadesh Kheria 2026-06-01 14:19:30 -07:00
parent 164dab6818
commit 03ed35e00a
2 changed files with 29 additions and 1 deletions

View File

@ -616,6 +616,28 @@ describe("ProjectOnboardingWizard", () => {
expect(screen.getByText("Payments setup is not available in remote development environments.")).toBeTruthy();
});
it("does not call setupPayments via Connect in a remote development environment, even if the disabled attribute is bypassed", async () => {
mockGetPublicEnvVar.mockImplementation((name: string) =>
name === "NEXT_PUBLIC_STACK_IS_REMOTE_DEVELOPMENT_ENVIRONMENT" ? "true" : "false"
);
const setupPayments = vi.fn(async () => ({ url: "https://example.com" }));
renderPaymentsSetupStep({ setupPayments });
// Simulate the disabled attribute being bypassed — DOM manipulation, a future
// refactor dropping the `disabled` prop, or any other way the button could
// be engaged despite the visible-state guard.
const connectButton = screen.getByRole("button", { name: "Connect" });
connectButton.removeAttribute("disabled");
fireEvent.click(connectButton);
// Flush microtasks so the async handler has a chance to run to completion.
await Promise.resolve();
await Promise.resolve();
expect(setupPayments).not.toHaveBeenCalled();
});
it("does not call setupPayments via Do Later in a remote development environment, even for a US project", async () => {
mockGetPublicEnvVar.mockImplementation((name: string) =>
name === "NEXT_PUBLIC_STACK_IS_REMOTE_DEVELOPMENT_ENVIRONMENT" ? "true" : "false"

View File

@ -393,6 +393,12 @@ export function ProjectOnboardingWizard(props: {
}, [isRemoteDevelopmentEnvironment, persistOnboardingState, props.project.app, runWithSaving, selectedPaymentsCountry, setStatus]);
const connectPaymentsSetup = useCallback(async () => {
// Defense-in-depth: the Connect button is disabled in RDE (the primary guard
// at the JSX level). This early-return is the secondary guard so the handler
// is safe even if the button is ever engaged some other way — a future refactor
// dropping the `disabled` prop, DOM manipulation, programmatic invocation, etc.
// Mirrors the symmetric guard in deferPaymentsSetup.
if (isRemoteDevelopmentEnvironment) return;
await runWithSaving(async () => {
setPaymentsSetupAction("connect");
try {
@ -406,7 +412,7 @@ export function ProjectOnboardingWizard(props: {
setPaymentsSetupAction(null);
}
});
}, [props.project.app, runWithSaving]);
}, [isRemoteDevelopmentEnvironment, props.project.app, runWithSaving]);
useEffect(() => {
if (status !== "payments_setup" || stripeAccountInfo?.details_submitted !== true || paymentsAutoCompletingRef.current) {