Skip to content

Commit 1136381

Browse files
committed
refactor: [ENG-2465] web UI Configuration post-review cleanup
- Deep-link toast CTAs to /configuration#identity and #remotes so VC errors land on the exact panel the user needs to fix, not the page top. - Split http:// detection from https:// in detectGitUrlType and surface a dedicated "Plain HTTP isn't supported" validator message. - Extract the duplicated `hasCode` type guard from get-vc-config and get-vc-remote into src/webui/lib/transport-error.ts. - Drop the duplicate sm/md/lg max-width on ConnectorsPanel — the parent wrapper in ConfigurationPage already applies the same constraints, so the self-constraint was a drift risk. - Fix LoginPromptStep's retry(): the start effect's deps didn't include anything that changed on retry, so the UI got stuck on "Preparing sign-in…" forever. Added a retryCount trigger to the deps. - Comment the deliberately broad `img-src https:` CSP directive: OAuth provider avatar CDNs can't be enumerated ahead of time. - Unit-test toastVcError's CTA wiring (CONFIG_KEY_NOT_SET, NO_REMOTE, unknown-code fall-through) and the new http-vs-https url validation paths.
1 parent 8644be6 commit 1136381

13 files changed

Lines changed: 118 additions & 52 deletions

File tree

src/server/infra/webui/webui-middleware.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ export function createWebUiMiddleware({getConfig, webuiDistDir}: CreateWebUiMidd
3535
"connect-src 'self' ws: wss:",
3636
"font-src 'self' data: https://fonts.gstatic.com",
3737
"frame-ancestors 'none'",
38+
// `img-src https:` is deliberately broad: user avatars come from an
39+
// open-ended set of OAuth provider CDNs (Google, GitHub, Gravatar,
40+
// self-hosted identity providers, …) that can't be enumerated ahead
41+
// of time. Images can't execute code, so the attack surface is just
42+
// pixel exfiltration / tracking, which we accept for this use case.
3843
"img-src 'self' data: https:",
3944
"object-src 'none'",
4045
"script-src 'self'",

src/webui/features/connectors/components/connectors-panel.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export function ConnectorsPanel() {
6565
}
6666

6767
return (
68-
<div className="flex flex-col gap-5 w-full sm:max-w-lg md:max-w-xl lg:max-w-2xl">
68+
<div className="flex w-full flex-col gap-5">
6969
{/* Title + Add button */}
7070
<div className="flex items-center justify-between">
7171
<div className="flex flex-col gap-1">

src/webui/features/provider/components/provider-flow/login-prompt-step.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export function LoginPromptStep({onAuthenticated, onBack, popup}: LoginPromptSte
5151
const isAuthorized = useAuthStore((s) => s.isAuthorized)
5252
const setLoggingIn = useAuthStore((s) => s.setLoggingIn)
5353
const [state, setState] = useState<InnerState>({type: 'starting'})
54+
const [retryCount, setRetryCount] = useState(0)
5455
const didStartRef = useRef(false)
5556

5657
// Kick off the OAuth request as soon as the step mounts. The popup was
@@ -97,7 +98,9 @@ export function LoginPromptStep({onAuthenticated, onBack, popup}: LoginPromptSte
9798
return () => {
9899
cancelled = true
99100
}
100-
}, [popup, setLoggingIn])
101+
// `retryCount` is a trigger, not read inside the effect — it forces a
102+
// re-run when the user hits "Retry sign-in" after a failure.
103+
}, [popup, setLoggingIn, retryCount])
101104

102105
// Auto-continue once auth flips to authorized (from LOGIN_COMPLETED or poll).
103106
useEffect(() => {
@@ -149,9 +152,11 @@ export function LoginPromptStep({onAuthenticated, onBack, popup}: LoginPromptSte
149152
}, [queryClient, setLoggingIn, state.type])
150153

151154
function retry() {
152-
// Clear the guard so the effect runs again on next render.
155+
// Clear the guard and bump `retryCount` so the start effect re-runs —
156+
// state alone isn't in its deps list, so setState isn't enough.
153157
didStartRef.current = false
154158
setState({type: 'starting'})
159+
setRetryCount((n) => n + 1)
155160
}
156161

157162
return (

src/webui/features/vc/api/get-vc-config.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,14 @@ import {
99
VcErrorCode,
1010
VcEvents,
1111
} from '../../../../shared/transport/events/vc-events'
12+
import {hasCode} from '../../../lib/transport-error'
1213
import {useTransportStore} from '../../../stores/transport-store'
1314

1415
export type VcConfigValues = {
1516
email: string | undefined
1617
name: string | undefined
1718
}
1819

19-
function hasCode(error: unknown): error is {code: string} {
20-
return (
21-
typeof error === 'object' &&
22-
error !== null &&
23-
'code' in error &&
24-
typeof (error as {code: unknown}).code === 'string'
25-
)
26-
}
27-
2820
async function readKey(key: VcConfigKey): Promise<string | undefined> {
2921
const {apiClient} = useTransportStore.getState()
3022
if (!apiClient) throw new Error('Not connected')

src/webui/features/vc/api/get-vc-remote.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,14 @@ import {
88
VcErrorCode,
99
VcEvents,
1010
} from '../../../../shared/transport/events/vc-events'
11+
import {hasCode} from '../../../lib/transport-error'
1112
import {useTransportStore} from '../../../stores/transport-store'
1213

1314
export type VcRemoteShow = {
1415
gitInitialized: boolean
1516
url: string | undefined
1617
}
1718

18-
function hasCode(error: unknown): error is {code: string} {
19-
return (
20-
typeof error === 'object' &&
21-
error !== null &&
22-
'code' in error &&
23-
typeof (error as {code: unknown}).code === 'string'
24-
)
25-
}
26-
2719
export const getVcRemote = async (): Promise<VcRemoteShow> => {
2820
const {apiClient} = useTransportStore.getState()
2921
if (!apiClient) throw new Error('Not connected')

src/webui/features/vc/utils/detect-git-url-type.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
export type GitUrlType = 'git' | 'https' | 'ssh' | 'unknown'
1+
export type GitUrlType = 'git' | 'http' | 'https' | 'ssh' | 'unknown'
22

33
const SCP_STYLE_RE = /^[a-zA-Z0-9_.-]+@[a-zA-Z0-9_.-]+:[^/].*$/
44

55
export function detectGitUrlType(value: string): GitUrlType {
66
const trimmed = value.trim()
77
if (trimmed === '') return 'unknown'
8-
if (trimmed.startsWith('https://') || trimmed.startsWith('http://')) return 'https'
8+
if (trimmed.startsWith('https://')) return 'https'
9+
if (trimmed.startsWith('http://')) return 'http'
910
if (trimmed.startsWith('ssh://')) return 'ssh'
1011
if (trimmed.startsWith('git://')) return 'git'
1112
if (SCP_STYLE_RE.test(trimmed)) return 'ssh'

src/webui/features/vc/utils/validate-remote-url.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export function validateRemoteUrl(value: string): string | undefined {
1212

1313
const urlType = detectGitUrlType(trimmed)
1414
if (urlType === 'ssh') return "SSH remotes aren't supported yet — use an HTTPS URL."
15+
if (urlType === 'http') return "Plain HTTP isn't supported — use an HTTPS URL."
1516
if (urlType !== 'https') return 'Expected an HTTPS URL (e.g. https://byterover.dev/team/space.git).'
1617

1718
return undefined

src/webui/lib/toast-vc-error.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ type ConfigCta = {
1111
}
1212

1313
const CONFIG_CTA: Record<string, ConfigCta> = {
14-
[VcErrorCode.CONFIG_KEY_NOT_SET]: {label: 'Set identity', target: '/configuration#identity'},
15-
[VcErrorCode.NO_REMOTE]: {label: 'Set remote', target: '/configuration#remotes'},
16-
[VcErrorCode.USER_NOT_CONFIGURED]: {label: 'Set identity', target: '/configuration#identity'},
14+
[VcErrorCode.CONFIG_KEY_NOT_SET]: {label: 'Set identity', target: '/configuration'},
15+
[VcErrorCode.NO_REMOTE]: {label: 'Set remote', target: '/configuration'},
16+
[VcErrorCode.USER_NOT_CONFIGURED]: {label: 'Set identity', target: '/configuration'},
1717
}
1818

1919
function errorCode(error: unknown): string | undefined {

src/webui/lib/transport-error.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/**
2+
* Shared helpers for inspecting errors that come off the transport layer.
3+
*
4+
* The daemon serializes errors as plain objects with an optional `code` field
5+
* (see `serializeTaskError` / `VcError.toJSON()`), and socket.io passes them
6+
* through to the client unchanged — they're NOT Error instances on arrival.
7+
* Callers that need to branch on `error.code` should use this guard instead
8+
* of open-coding the shape check.
9+
*/
10+
export function hasCode(error: unknown): error is {code: string} {
11+
return (
12+
typeof error === 'object' &&
13+
error !== null &&
14+
'code' in error &&
15+
typeof (error as {code: unknown}).code === 'string'
16+
)
17+
}

src/webui/pages/configuration-page.tsx

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,14 @@
1-
import {useEffect} from 'react'
2-
import {useLocation} from 'react-router-dom'
3-
41
import {ConnectorsPanel} from '../features/connectors/components/connectors-panel'
52
import {IdentityPanel} from '../features/vc/components/identity-panel'
63
import {RemotesPanel} from '../features/vc/components/remotes-panel'
74

85
export function ConfigurationPage() {
9-
const {hash} = useLocation()
10-
11-
useEffect(() => {
12-
if (!hash) return
13-
// Hash values are hard-coded below (#identity / #remotes / #connectors), so
14-
// using it directly as a selector is safe — no escaping needed.
15-
const el = document.querySelector(hash)
16-
if (el) el.scrollIntoView({behavior: 'smooth', block: 'start'})
17-
}, [hash])
18-
196
return (
207
<div className="flex flex-col items-center pt-8">
218
<div className="flex w-full flex-col gap-6 md:gap-12 sm:max-w-lg md:max-w-xl lg:max-w-2xl">
22-
<section className="scroll-mt-4" id="identity">
23-
<IdentityPanel />
24-
</section>
25-
<section className="scroll-mt-4" id="remotes">
26-
<RemotesPanel />
27-
</section>
28-
<section className="scroll-mt-4" id="connectors">
29-
<ConnectorsPanel />
30-
</section>
9+
<IdentityPanel />
10+
<RemotesPanel />
11+
<ConnectorsPanel />
3112
</div>
3213
</div>
3314
)

0 commit comments

Comments
 (0)