Skip to content

Commit 650a104

Browse files
waleedlatif1emir-karabeg
authored andcommitted
fix(secrets): restore unsaved-changes guard for settings tab navigation (#4009)
* fix(secrets): restore unsaved-changes guard for settings tab navigation - Add useSettingsDirtyStore (stores/settings/dirty) to track dirty state across the settings sidebar and section components - Wire credentials-manager and integrations-manager to sync dirty state to the store and clean up on unmount; also reset store synchronously in handleDiscardAndNavigate - Update settings-sidebar to check dirty state before tab switches and Back navigation, showing an Unsaved Changes dialog if needed - Remove dead stores/settings/environment directory; move EnvironmentVariable type into lib/environment/api * fix(teams): harden Microsoft content URL validation - Add isMicrosoftContentUrl helper with typed allowlist covering SharePoint, OneDrive, and Teams CDN domains - Replace loose substring checks in Teams webhook handler with parsed-hostname matching to prevent bypass via partial domain names - Deduplicate OneDrive share-link detection into isOneDriveShareLink flag and use searchParams API instead of string splitting * fix(env): remove type re-exports from query file, drop keepPreviousData on static key * fix(teams): remove smba.trafficmanager.net from Microsoft content allowlist The subdomain check for smba.trafficmanager.net was unnecessary — Azure Traffic Manager does not support nested subdomains of existing profiles, but the pattern still raised a valid audit concern. Teams bot-framework attachment URLs from this host fall through to the generic fetchWithDNSPinning branch, which provides the same protection without the ambiguity. * fix(secrets): guard active-tab re-click, restore keepPreviousData on workspace env query * fix(teams): add 1drv.com apex to OneDrive share-link branch 1drv.com (apex) is a short-link domain functionally equivalent to 1drv.ms and requires share-token resolution, not direct fetch. CDN subdomains (files.1drv.com) are unaffected — the exact-match check leaves them on the direct-fetch path.
1 parent b4d9057 commit 650a104

File tree

13 files changed

+217
-75
lines changed

13 files changed

+217
-75
lines changed

apps/sim/app/api/environment/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
1010
import { generateRequestId } from '@/lib/core/utils/request'
1111
import { generateId } from '@/lib/core/utils/uuid'
1212
import { syncPersonalEnvCredentialsForUser } from '@/lib/credentials/environment'
13-
import type { EnvironmentVariable } from '@/stores/settings/environment'
13+
import type { EnvironmentVariable } from '@/lib/environment/api'
1414

1515
const logger = createLogger('EnvironmentAPI')
1616

apps/sim/app/workspace/[workspaceId]/settings/components/credentials/credentials-manager.tsx

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
type PendingCredentialCreateRequest,
3131
readPendingCredentialCreateRequest,
3232
} from '@/lib/credentials/client-state'
33+
import type { WorkspaceEnvironmentData } from '@/lib/environment/api'
3334
import { getUserColor } from '@/lib/workspaces/colors'
3435
import { isValidEnvVarName } from '@/executor/constants'
3536
import {
@@ -48,9 +49,9 @@ import {
4849
useSavePersonalEnvironment,
4950
useUpsertWorkspaceEnvironment,
5051
useWorkspaceEnvironment,
51-
type WorkspaceEnvironmentData,
5252
} from '@/hooks/queries/environment'
5353
import { useWorkspacePermissionsQuery } from '@/hooks/queries/workspace'
54+
import { useSettingsDirtyStore } from '@/stores/settings/dirty/store'
5455

5556
const logger = createLogger('SecretsManager')
5657

@@ -482,6 +483,15 @@ export function CredentialsManager() {
482483
hasChangesRef.current = hasChanges
483484
shouldBlockNavRef.current = hasChanges || isDetailsDirty
484485

486+
const setNavGuardDirty = useSettingsDirtyStore((s) => s.setDirty)
487+
const resetNavGuard = useSettingsDirtyStore((s) => s.reset)
488+
489+
useEffect(() => {
490+
setNavGuardDirty(hasChanges || isDetailsDirty)
491+
}, [hasChanges, isDetailsDirty, setNavGuardDirty])
492+
493+
useEffect(() => () => resetNavGuard(), [resetNavGuard])
494+
485495
// --- Effects ---
486496
useEffect(() => {
487497
if (hasSavedRef.current) return
@@ -981,6 +991,7 @@ export function CredentialsManager() {
981991

982992
const handleDiscardAndNavigate = useCallback(() => {
983993
shouldBlockNavRef.current = false
994+
resetNavGuard()
984995
resetToSaved()
985996
setSelectedCredentialId(null)
986997

@@ -989,7 +1000,7 @@ export function CredentialsManager() {
9891000
pendingNavigationUrlRef.current = null
9901001
router.push(url)
9911002
}
992-
}, [router, resetToSaved])
1003+
}, [router, resetToSaved, resetNavGuard])
9931004

9941005
const renderEnvVarRow = useCallback(
9951006
(envVar: UIEnvironmentVariable, originalIndex: number) => {

apps/sim/app/workspace/[workspaceId]/settings/components/integrations/integrations-manager.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import {
5454
} from '@/hooks/queries/oauth/oauth-connections'
5555
import { useWorkspacePermissionsQuery } from '@/hooks/queries/workspace'
5656
import { useOAuthReturnRouter } from '@/hooks/use-oauth-return'
57+
import { useSettingsDirtyStore } from '@/stores/settings/dirty/store'
5758

5859
const logger = createLogger('IntegrationsManager')
5960

@@ -247,6 +248,15 @@ export function IntegrationsManager() {
247248

248249
const isDetailsDirty = isDescriptionDirty || isDisplayNameDirty
249250

251+
const setNavGuardDirty = useSettingsDirtyStore((s) => s.setDirty)
252+
const resetNavGuard = useSettingsDirtyStore((s) => s.reset)
253+
254+
useEffect(() => {
255+
setNavGuardDirty(isDetailsDirty)
256+
}, [isDetailsDirty, setNavGuardDirty])
257+
258+
useEffect(() => () => resetNavGuard(), [resetNavGuard])
259+
250260
const handleSaveDetails = async () => {
251261
if (!selectedCredential || !isSelectedAdmin || !isDetailsDirty || updateCredential.isPending)
252262
return

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/env-var-dropdown.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,8 @@ import {
1010
} from '@/components/emcn'
1111
import { cn } from '@/lib/core/utils/cn'
1212
import { writePendingCredentialCreateRequest } from '@/lib/credentials/client-state'
13-
import {
14-
usePersonalEnvironment,
15-
useWorkspaceEnvironment,
16-
type WorkspaceEnvironmentData,
17-
} from '@/hooks/queries/environment'
13+
import type { WorkspaceEnvironmentData } from '@/lib/environment/api'
14+
import { usePersonalEnvironment, useWorkspaceEnvironment } from '@/hooks/queries/environment'
1815
import { useSettingsNavigation } from '@/hooks/use-settings-navigation'
1916

2017
/**

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-sidebar/settings-sidebar.tsx

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
11
'use client'
22

3-
import { useCallback, useMemo } from 'react'
3+
import { useCallback, useMemo, useState } from 'react'
44
import { useQueryClient } from '@tanstack/react-query'
55
import { useParams, usePathname, useRouter } from 'next/navigation'
6-
import { ChevronDown, Skeleton } from '@/components/emcn'
6+
import {
7+
Button,
8+
ChevronDown,
9+
Modal,
10+
ModalBody,
11+
ModalContent,
12+
ModalFooter,
13+
ModalHeader,
14+
Skeleton,
15+
} from '@/components/emcn'
716
import { useSession } from '@/lib/auth/auth-client'
817
import { getSubscriptionAccessState } from '@/lib/billing/client'
918
import { isHosted } from '@/lib/core/config/feature-flags'
@@ -23,6 +32,7 @@ import { useOrganizations } from '@/hooks/queries/organization'
2332
import { prefetchSubscriptionData, useSubscriptionData } from '@/hooks/queries/subscription'
2433
import { usePermissionConfig } from '@/hooks/use-permission-config'
2534
import { useSettingsNavigation } from '@/hooks/use-settings-navigation'
35+
import { useSettingsDirtyStore } from '@/stores/settings/dirty/store'
2636

2737
const SKELETON_SECTIONS = [3, 2, 2] as const
2838

@@ -41,6 +51,13 @@ export function SettingsSidebar({
4151
const router = useRouter()
4252

4353
const queryClient = useQueryClient()
54+
55+
const requestNavigation = useSettingsDirtyStore((s) => s.requestNavigation)
56+
const confirmNavigation = useSettingsDirtyStore((s) => s.confirmNavigation)
57+
const cancelNavigation = useSettingsDirtyStore((s) => s.cancelNavigation)
58+
const isDirty = useSettingsDirtyStore((s) => s.isDirty)
59+
const [showDiscardDialog, setShowDiscardDialog] = useState(false)
60+
4461
const { data: session, isPending: sessionLoading } = useSession()
4562
const { data: organizationsData, isLoading: orgsLoading } = useOrganizations()
4663
const { data: generalSettings } = useGeneralSettings()
@@ -180,8 +197,27 @@ export function SettingsSidebar({
180197
const { popSettingsReturnUrl, getSettingsHref } = useSettingsNavigation()
181198

182199
const handleBack = useCallback(() => {
200+
if (isDirty) {
201+
setShowDiscardDialog(true)
202+
return
203+
}
183204
router.push(popSettingsReturnUrl(`/workspace/${workspaceId}/home`))
184-
}, [router, popSettingsReturnUrl, workspaceId])
205+
}, [router, popSettingsReturnUrl, workspaceId, isDirty])
206+
207+
const handleConfirmDiscard = useCallback(() => {
208+
const section = confirmNavigation()
209+
setShowDiscardDialog(false)
210+
if (section) {
211+
router.replace(getSettingsHref({ section }), { scroll: false })
212+
} else {
213+
router.push(popSettingsReturnUrl(`/workspace/${workspaceId}/home`))
214+
}
215+
}, [confirmNavigation, router, getSettingsHref, popSettingsReturnUrl, workspaceId])
216+
217+
const handleCancelDiscard = useCallback(() => {
218+
cancelNavigation()
219+
setShowDiscardDialog(false)
220+
}, [cancelNavigation])
185221

186222
return (
187223
<>
@@ -286,11 +322,15 @@ export function SettingsSidebar({
286322
className={itemClassName}
287323
onMouseEnter={() => handlePrefetch(item.id)}
288324
onFocus={() => handlePrefetch(item.id)}
289-
onClick={() =>
290-
router.replace(getSettingsHref({ section: item.id as SettingsSection }), {
291-
scroll: false,
292-
})
293-
}
325+
onClick={() => {
326+
const section = item.id as SettingsSection
327+
if (section === activeSection) return
328+
if (!requestNavigation(section)) {
329+
setShowDiscardDialog(true)
330+
return
331+
}
332+
router.replace(getSettingsHref({ section }), { scroll: false })
333+
}}
294334
>
295335
{content}
296336
</button>
@@ -312,6 +352,25 @@ export function SettingsSidebar({
312352
})
313353
)}
314354
</div>
355+
356+
<Modal open={showDiscardDialog} onOpenChange={(open) => !open && handleCancelDiscard()}>
357+
<ModalContent size='sm'>
358+
<ModalHeader>Unsaved Changes</ModalHeader>
359+
<ModalBody>
360+
<p className='text-[var(--text-secondary)]'>
361+
You have unsaved changes. Are you sure you want to discard them?
362+
</p>
363+
</ModalBody>
364+
<ModalFooter>
365+
<Button variant='default' onClick={handleCancelDiscard}>
366+
Keep Editing
367+
</Button>
368+
<Button variant='destructive' onClick={handleConfirmDiscard}>
369+
Discard Changes
370+
</Button>
371+
</ModalFooter>
372+
</ModalContent>
373+
</Modal>
315374
</>
316375
)
317376
}

apps/sim/hooks/queries/environment.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,9 @@
11
import { createLogger } from '@sim/logger'
22
import { keepPreviousData, useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
3-
import type { WorkspaceEnvironmentData } from '@/lib/environment/api'
3+
import type { EnvironmentVariable, WorkspaceEnvironmentData } from '@/lib/environment/api'
44
import { fetchPersonalEnvironment, fetchWorkspaceEnvironment } from '@/lib/environment/api'
55
import { workspaceCredentialKeys } from '@/hooks/queries/credentials'
66
import { API_ENDPOINTS } from '@/stores/constants'
7-
import type { EnvironmentVariable } from '@/stores/settings/environment'
8-
9-
export type { WorkspaceEnvironmentData } from '@/lib/environment/api'
10-
export type { EnvironmentVariable } from '@/stores/settings/environment'
117

128
const logger = createLogger('EnvironmentQueries')
139

@@ -27,8 +23,7 @@ export function usePersonalEnvironment() {
2723
return useQuery({
2824
queryKey: environmentKeys.personal(),
2925
queryFn: ({ signal }) => fetchPersonalEnvironment(signal),
30-
staleTime: 60 * 1000, // 1 minute
31-
placeholderData: keepPreviousData,
26+
staleTime: 60 * 1000,
3227
})
3328
}
3429

apps/sim/lib/core/security/input-validation.ts

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -741,18 +741,8 @@ export function validateExternalUrl(
741741
}
742742
}
743743

744-
// Block suspicious ports commonly used for internal services
745744
const port = parsedUrl.port
746-
const blockedPorts = [
747-
'22', // SSH
748-
'23', // Telnet
749-
'25', // SMTP
750-
'3306', // MySQL
751-
'5432', // PostgreSQL
752-
'6379', // Redis
753-
'27017', // MongoDB
754-
'9200', // Elasticsearch
755-
]
745+
const blockedPorts = ['22', '23', '25', '3306', '5432', '6379', '27017', '9200']
756746

757747
if (port && blockedPorts.includes(port)) {
758748
return {
@@ -842,7 +832,6 @@ export function validateAirtableId(
842832
}
843833
}
844834

845-
// Airtable IDs: prefix (3 chars) + 14 alphanumeric characters = 17 chars total
846835
const airtableIdPattern = new RegExp(`^${expectedPrefix}[a-zA-Z0-9]{14}$`)
847836

848837
if (!airtableIdPattern.test(value)) {
@@ -893,11 +882,6 @@ export function validateAwsRegion(
893882
}
894883
}
895884

896-
// AWS region patterns:
897-
// - Standard: af|ap|ca|eu|me|sa|us|il followed by direction and number
898-
// - GovCloud: us-gov-east-1, us-gov-west-1
899-
// - China: cn-north-1, cn-northwest-1
900-
// - ISO: us-iso-east-1, us-iso-west-1, us-isob-east-1
901885
const awsRegionPattern =
902886
/^(af|ap|ca|cn|eu|il|me|sa|us|us-gov|us-iso|us-isob)-(central|north|northeast|northwest|south|southeast|southwest|east|west)-\d{1,2}$/
903887

@@ -1156,7 +1140,6 @@ export function validatePaginationCursor(
11561140
}
11571141
}
11581142

1159-
// Allow alphanumeric, base64 chars (+, /, =), and URL-safe chars (-, _, ., ~, %)
11601143
const cursorPattern = /^[A-Za-z0-9+/=\-_.~%]+$/
11611144
if (!cursorPattern.test(value)) {
11621145
logger.warn('Pagination cursor contains disallowed characters', {
@@ -1224,3 +1207,43 @@ export function validateOktaDomain(rawDomain: string): string {
12241207
}
12251208
return domain
12261209
}
1210+
1211+
const MICROSOFT_CONTENT_SUFFIXES = [
1212+
'sharepoint.com',
1213+
'sharepoint.us',
1214+
'sharepoint.de',
1215+
'sharepoint.cn',
1216+
'sharepointonline.com',
1217+
'onedrive.com',
1218+
'onedrive.live.com',
1219+
'1drv.ms',
1220+
'1drv.com',
1221+
'microsoftpersonalcontent.com',
1222+
] as const
1223+
1224+
/**
1225+
* Returns true if the given URL is hosted on a trusted Microsoft SharePoint or
1226+
* OneDrive domain. Validates the parsed hostname against an allowlist using exact
1227+
* match or subdomain suffix, preventing incomplete-substring bypasses.
1228+
*
1229+
* Covers SharePoint Online (commercial, GCC/GCC High/DoD, Germany, China),
1230+
* OneDrive business and consumer, OneDrive short-link and CDN domains,
1231+
* and Microsoft personal content CDN.
1232+
*
1233+
* @see https://learn.microsoft.com/en-us/sharepoint/required-urls-and-ports
1234+
* @see https://learn.microsoft.com/en-us/microsoft-365/enterprise/microsoft-365-u-s-government-gcc-high-endpoints
1235+
*
1236+
* @param url - The URL to check
1237+
* @returns Whether the URL belongs to a trusted Microsoft content host
1238+
*/
1239+
export function isMicrosoftContentUrl(url: string): boolean {
1240+
let hostname: string
1241+
try {
1242+
hostname = new URL(url).hostname.toLowerCase()
1243+
} catch {
1244+
return false
1245+
}
1246+
return MICROSOFT_CONTENT_SUFFIXES.some(
1247+
(suffix) => hostname === suffix || hostname.endsWith(`.${suffix}`)
1248+
)
1249+
}

apps/sim/lib/environment/api.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { API_ENDPOINTS } from '@/stores/constants'
2-
import type { EnvironmentVariable } from '@/stores/settings/environment'
2+
3+
export interface EnvironmentVariable {
4+
key: string
5+
value: string
6+
}
37

48
export interface WorkspaceEnvironmentData {
59
workspace: Record<string, string>

0 commit comments

Comments
 (0)