diff --git a/apps/ui/src/features/deploy/github-deployer/github-deployer.context.tsx b/apps/ui/src/features/deploy/github-deployer/github-deployer.context.tsx index b54a6359..21278458 100644 --- a/apps/ui/src/features/deploy/github-deployer/github-deployer.context.tsx +++ b/apps/ui/src/features/deploy/github-deployer/github-deployer.context.tsx @@ -6,7 +6,9 @@ import { type ReactNode, useCallback, useContext, + useEffect, useMemo, + useRef, useState, } from "react"; @@ -15,6 +17,7 @@ import { findTemplateForGithubRepo, templateCanDeployWithDefaults, } from "@/features/deploy/github/github-template-match"; +import { normalizeGithubRepoUrl } from "../github-repo-url"; import { templateSensitiveKeys } from "../template-deployer"; import type { GithubDeployerActions, @@ -50,6 +53,36 @@ export function GithubDeployerRoot({ initialRepoUrl?: string; states: GithubDeployerStates; }) { + const autoAuthorizeAttemptedRef = useRef(false); + const { onAutoAuthorize } = actions; + const { deployedRepo, isAuthorized, isLoading } = states; + const requestedRepoUrl = normalizeGithubRepoUrl(initialRepoUrl); + + useEffect(() => { + if ( + !autoDeploy || + requestedRepoUrl == null || + deployedRepo || + isAuthorized || + isLoading || + onAutoAuthorize == null || + autoAuthorizeAttemptedRef.current + ) { + return; + } + // Root remains mounted while Shell hides the repository input before authorization. + // Closing or blocking the popup must not start another authorization loop. + autoAuthorizeAttemptedRef.current = true; + onAutoAuthorize(); + }, [ + autoDeploy, + deployedRepo, + isAuthorized, + isLoading, + onAutoAuthorize, + requestedRepoUrl, + ]); + const [selectedRepoId, setSelectedRepoId] = useState( () => states.repos[0]?.id ?? "" ); @@ -73,12 +106,14 @@ export function GithubDeployerRoot({ const resolvedActions = useMemo( () => ({ + onAutoAuthorize: actions.onAutoAuthorize, onAuthorize: actions.onAuthorize, onDisconnect: actions.onDisconnect, onDeploy: actions.onDeploy, onDeployTemplate: actions.onDeployTemplate, }), [ + actions.onAutoAuthorize, actions.onAuthorize, actions.onDeploy, actions.onDeployTemplate, diff --git a/apps/ui/src/features/deploy/github-deployer/github-deployer.test.tsx b/apps/ui/src/features/deploy/github-deployer/github-deployer.test.tsx index 4e87c9d6..0a6dbc65 100644 --- a/apps/ui/src/features/deploy/github-deployer/github-deployer.test.tsx +++ b/apps/ui/src/features/deploy/github-deployer/github-deployer.test.tsx @@ -183,11 +183,17 @@ test("GithubDeployer auto deploys a restored GitHub URL only once", async () => const dom = installTestDom(); const previousActEnvironment = setActEnvironment(true); let deployCalls = 0; + let authorizeCalls = 0; const onDeploy = () => { deployCalls += 1; }; const initialProps = { - actions: { onDeploy }, + actions: { + onAutoAuthorize: () => { + authorizeCalls += 1; + }, + onDeploy, + }, autoDeploy: true, initialRepoUrl: "https://github.com/acme/api", states: { @@ -207,25 +213,37 @@ test("GithubDeployer auto deploys a restored GitHub URL only once", async () => await actAndDrain(() => { rendered = render( - + ); }); assert.equal(deployCalls, 0); + assert.equal(authorizeCalls, 1); + + // A cancelled authorization must leave the manual button available without retrying. + await actAndDrain(() => { + rendered?.rerender( + + + + ); + }); + assert.equal(authorizeCalls, 1); await actAndDrain(() => { rendered?.rerender( - + ); }); assert.equal(deployCalls, 1); + assert.equal(authorizeCalls, 1); await actAndDrain(() => { rendered?.rerender( - + ); }); @@ -241,6 +259,80 @@ test("GithubDeployer auto deploys a restored GitHub URL only once", async () => } }); +test("GithubDeployer only auto-connects a valid deployment after auth readiness", async () => { + const dom = installTestDom(); + const previousActEnvironment = setActEnvironment(true); + let authorizeCalls = 0; + const onAutoAuthorize = () => { + authorizeCalls += 1; + }; + let rendered: ReturnType | undefined; + try { + const cases = [ + { autoDeploy: false, initialRepoUrl: "https://github.com/acme/api" }, + { autoDeploy: true, initialRepoUrl: "https://example.com/acme/api" }, + { autoDeploy: true, initialRepoUrl: "" }, + { + autoDeploy: true, + initialRepoUrl: "https://github.com/acme/api", + isAuthorized: true, + }, + { + autoDeploy: true, + initialRepoUrl: "https://github.com/acme/api", + isLoading: true, + }, + ]; + for (const entry of cases) { + await actAndDrain(() => { + rendered = render( + + + + ); + }); + assert.equal(authorizeCalls, 0); + await actAndDrain(() => rendered?.unmount()); + } + + // The host withholds the automatic action until credentials and auth lookup are ready. + const props = { + autoDeploy: true, + initialRepoUrl: "https://github.com/acme/api", + states: { isAuthorized: false, repos: [] }, + } as const; + await actAndDrain(() => { + rendered = render( + + + + ); + }); + assert.equal(authorizeCalls, 0); + await actAndDrain(() => { + rendered?.rerender( + + + + ); + }); + assert.equal(authorizeCalls, 1); + } finally { + await actAndDrain(() => rendered?.unmount()); + restoreActEnvironment(previousActEnvironment); + await dom.restore(); + } +}); + test("GithubDeployer auto deploy stays one-shot after the URL is edited", async () => { const dom = installTestDom(); const previousActEnvironment = setActEnvironment(true); diff --git a/apps/ui/src/features/deploy/github-deployer/github-deployer.types.ts b/apps/ui/src/features/deploy/github-deployer/github-deployer.types.ts index 09751da6..8da28abd 100644 --- a/apps/ui/src/features/deploy/github-deployer/github-deployer.types.ts +++ b/apps/ui/src/features/deploy/github-deployer/github-deployer.types.ts @@ -55,6 +55,8 @@ export interface GithubDeployerStates { export interface GithubDeployerActions { /** Invoked when the user connects or reconfigures workspace GitHub access. */ onAuthorize?: () => void; + /** Starts authorization for a restored one-click deployment, once auth status is known. */ + onAutoAuthorize?: () => void; /** Invoked when Deploy is pressed with the selected repo. */ onDeploy?: (repo: GithubDeployerRepo) => void | Promise; /** Invoked when the user accepts a matched app-store template recommendation. */ @@ -69,6 +71,7 @@ export interface GithubDeployerActions { export interface GithubDeployerResolvedActions { onAuthorize?: () => void; + onAutoAuthorize?: () => void; onDeploy?: (repo: GithubDeployerRepo) => void | Promise; onDeployTemplate?: GithubDeployerActions["onDeployTemplate"]; onDisconnect?: () => void; diff --git a/apps/ui/src/features/deploy/github/types.test.ts b/apps/ui/src/features/deploy/github/types.test.ts index 3bd64310..909fee09 100644 --- a/apps/ui/src/features/deploy/github/types.test.ts +++ b/apps/ui/src/features/deploy/github/types.test.ts @@ -1,5 +1,6 @@ import assert from "node:assert/strict"; import { test } from "node:test"; +import { githubDeployProjectPath } from "../github-deploy-link"; import { parseInstallNamespaceParam, parseInstallReturnPathParam, @@ -24,4 +25,29 @@ test("parseInstallReturnPathParam rejects paths that browsers may treat as exter assert.equal(parseInstallReturnPathParam("/projects"), "/projects"); assert.equal(parseInstallReturnPathParam("/%5Cevil.example"), null); assert.equal(parseInstallReturnPathParam("/\\evil.example"), null); + for (const path of [ + "https://evil.example", + "//evil.example", + "/%2Fevil.example", + "/\t/evil.example", + "/%0a/evil.example", + ]) { + assert.equal(parseInstallReturnPathParam(path), null); + } +}); + +test("GitHub deployment return paths survive every OAuth validation unchanged", () => { + const path = githubDeployProjectPath("https://github.com/zjy365/aster", "1"); + let returnPath: string | null = path; + // Session creation, callback URL construction, completion page, opener message. + for (let step = 0; step < 4; step += 1) { + returnPath = parseInstallReturnPathParam(returnPath); + assert.equal(returnPath, path); + } +}); + +test("return paths permit repository URLs in query values without decoding them", () => { + const path = + "/deploy?githubRepo=https%3A%2F%2Fgithub.com%2Fzjy365%2Faster&autoDeploy=1"; + assert.equal(parseInstallReturnPathParam(path), path); }); diff --git a/apps/ui/src/features/deploy/github/types.ts b/apps/ui/src/features/deploy/github/types.ts index 521b0398..d099c831 100644 --- a/apps/ui/src/features/deploy/github/types.ts +++ b/apps/ui/src/features/deploy/github/types.ts @@ -26,6 +26,7 @@ export const MAX_INSTALL_RETURN_PATH_LEN = 2048; /** Conservative namespace format accepted from the browser during install. */ const INSTALL_NAMESPACE_RE = /^[a-z0-9]([-a-z0-9]*[a-z0-9])?$/; +const RETURN_PATH_SUFFIX_RE = /[?#]/; /** Max length for Kubernetes namespace names. */ const MAX_INSTALL_NAMESPACE_LEN = 63; @@ -38,25 +39,32 @@ export function parseInstallReturnPathParam(raw: string | null): string | null { if (raw == null || raw === "") { return null; } + const value = raw.trim(); + if (value.length > MAX_INSTALL_RETURN_PATH_LEN || !value.startsWith("/")) { + return null; + } let decoded: string; try { - decoded = decodeURIComponent(raw); + decoded = decodeURIComponent(value); } catch { return null; } - decoded = decoded.trim(); + const pathname = decoded.split(RETURN_PATH_SUFFIX_RE, 1)[0] ?? ""; if ( decoded.length > MAX_INSTALL_RETURN_PATH_LEN || !decoded.startsWith("/") || decoded.startsWith("//") || decoded.includes("\\") || - decoded.includes("://") || + pathname.includes("://") || + decoded.includes("\t") || decoded.includes("\n") || decoded.includes("\r") ) { return null; } - return decoded; + // JSON and URLSearchParams already decode the transport layer. Preserve query + // encoding: nested pane URLs must survive every OAuth callback validation. + return value; } /** diff --git a/apps/ui/src/features/deploy/github/use-github-auth.test.ts b/apps/ui/src/features/deploy/github/use-github-auth.test.ts index 427d6437..2587bc77 100644 --- a/apps/ui/src/features/deploy/github/use-github-auth.test.ts +++ b/apps/ui/src/features/deploy/github/use-github-auth.test.ts @@ -1,9 +1,28 @@ import assert from "node:assert/strict"; import { test } from "node:test"; +import { githubDeployProjectPath } from "../github-deploy-link"; import { GITHUB_APP_INSTALL_COMPLETE_MESSAGE } from "./types"; import { githubInstallReturnPathForNavigation } from "./use-github-auth"; +test("OAuth completion preserves the original one-click deployment pane", () => { + const returnPath = githubDeployProjectPath( + "https://github.com/zjy365/aster", + "1" + ); + assert.equal( + githubInstallReturnPathForNavigation( + { + returnPath, + state: "install-state", + type: GITHUB_APP_INSTALL_COMPLETE_MESSAGE, + }, + { applyReturnPath: true } + ), + returnPath + ); +}); + test("githubInstallReturnPathForNavigation only applies targeted return paths", () => { const message = { returnPath: "/projects?source=github", diff --git a/apps/ui/src/features/deploy/github/use-github-auth.ts b/apps/ui/src/features/deploy/github/use-github-auth.ts index f0fc49d6..46ee1850 100644 --- a/apps/ui/src/features/deploy/github/use-github-auth.ts +++ b/apps/ui/src/features/deploy/github/use-github-auth.ts @@ -43,7 +43,7 @@ export interface UseGithubAuthResult { disconnectGithubAuth: () => Promise; error: Error | undefined; githubLogin: string | undefined; - initiateGithubAuth: () => void; + initiateGithubAuth: (options?: { automatic?: boolean }) => void; isAuthorized: boolean; isLoading: boolean; mutate: () => Promise; @@ -303,67 +303,78 @@ export function useGithubAuth(options?: { }; }, [canCheck, handleInstallComplete]); - const initiateGithubAuth = useCallback(() => { - const next = `${window.location.pathname}${window.location.search}`; - const normalizedNamespace = parseInstallNamespaceParam(namespace); - const openPopup = () => { - const popup = window.open( - "about:blank", - GITHUB_APP_INSTALL_POPUP_NAME, - centeredPopupFeatures() - ); - if (!popup) { - return null; - } - popup.focus(); - return popup; - }; + const initiateGithubAuth = useCallback( + (options?: { automatic?: boolean }) => { + const next = `${window.location.pathname}${window.location.search}`; + const normalizedNamespace = parseInstallNamespaceParam(namespace); + const openPopup = () => { + const popup = window.open( + "about:blank", + GITHUB_APP_INSTALL_POPUP_NAME, + centeredPopupFeatures() + ); + if (!popup) { + return null; + } + popup.focus(); + return popup; + }; - let closePoll: number | undefined; - const cleanup = () => { - if (closePoll !== undefined) { - window.clearInterval(closePoll); - closePoll = undefined; - } - if (installCleanupRef.current === cleanup) { - installCleanupRef.current = null; - } - pendingInstallStateRef.current = null; - }; - installCleanupRef.current = cleanup; + let closePoll: number | undefined; + const cleanup = () => { + if (closePoll !== undefined) { + window.clearInterval(closePoll); + closePoll = undefined; + } + if (installCleanupRef.current === cleanup) { + installCleanupRef.current = null; + } + pendingInstallStateRef.current = null; + }; + installCleanupRef.current = cleanup; - const start = async (popup: Window | null) => { - if (kubeconfig.trim() === "" || normalizedNamespace == null) { - throw new Error("GitHub authorization requires workspace credentials."); - } - const { authorizeUrl, state } = await createOAuthSession( - { appToken, kubeconfig, namespace: normalizedNamespace }, - next - ); - pendingInstallStateRef.current = state; - handledInstallStatesRef.current.delete(state); - if (popup == null) { - window.location.assign(authorizeUrl); - return; - } - popup.location.replace(authorizeUrl); - closePoll = window.setInterval(() => { - if (popup.closed) { - cleanup(); - refreshConnection(); + const start = async (popup: Window | null) => { + if (kubeconfig.trim() === "" || normalizedNamespace == null) { + throw new Error( + "GitHub authorization requires workspace credentials." + ); } - }, 1000); - }; + const { authorizeUrl, state } = await createOAuthSession( + { appToken, kubeconfig, namespace: normalizedNamespace }, + next + ); + pendingInstallStateRef.current = state; + handledInstallStatesRef.current.delete(state); + if (popup == null) { + window.location.assign(authorizeUrl); + return; + } + popup.location.replace(authorizeUrl); + closePoll = window.setInterval(() => { + if (popup.closed) { + cleanup(); + refreshConnection(); + } + }, 1000); + }; - const popup = openPopup(); - start(popup).catch((startError: unknown) => { - popup?.close(); - console.error( - "[github-auth] failed to create OAuth session:", - startError - ); - }); - }, [appToken, kubeconfig, namespace, refreshConnection]); + const popup = openPopup(); + // An automatic effect has no user activation. Keep the manual Connect button + // available if blocked, rather than navigating an embedded Brain to GitHub. + if (popup == null && options?.automatic) { + cleanup(); + return; + } + start(popup).catch((startError: unknown) => { + popup?.close(); + console.error( + "[github-auth] failed to create OAuth session:", + startError + ); + }); + }, + [appToken, kubeconfig, namespace, refreshConnection] + ); const disconnectGithubAuth = useCallback(async () => { if (!canCheck) { diff --git a/apps/ui/src/features/projects/creation/use-project-creator.ts b/apps/ui/src/features/projects/creation/use-project-creator.ts index 874017d3..4307a001 100644 --- a/apps/ui/src/features/projects/creation/use-project-creator.ts +++ b/apps/ui/src/features/projects/creation/use-project-creator.ts @@ -159,6 +159,8 @@ export function useProjectCreator(options?: UseProjectCreatorOptions): { }); const { + canCheck: canCheckGithubAuth, + error: githubAuthError, disconnectGithubAuth, initiateGithubAuth, isAuthorized: githubAuthorized, @@ -478,6 +480,10 @@ export function useProjectCreator(options?: UseProjectCreatorOptions): { const githubDeployer = useMemo( () => ({ actions: { + onAutoAuthorize: + canCheckGithubAuth && !githubAuthLoading && !githubAuthError + ? () => initiateGithubAuth({ automatic: true }) + : undefined, onAuthorize: initiateGithubAuth, onDisconnect: handleGithubDisconnect, onDeploy: handleGithubDeploy, @@ -497,6 +503,9 @@ export function useProjectCreator(options?: UseProjectCreatorOptions): { }, }), [ + canCheckGithubAuth, + githubAuthError, + githubAuthLoading, githubDeployerLoading, githubReposError, githubRepos,