Skip to content

Commit 2b1d31a

Browse files
committed
fix(extension): bound link drafts and stabilize browser QA
1 parent e69815a commit 2b1d31a

5 files changed

Lines changed: 106 additions & 22 deletions

File tree

‎apps/extension/scripts/test-e2e.ts‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,74 @@ try {
378378
}
379379
)
380380

381+
await check(
382+
'Link drafts enforce the saved URL limit after removing private URL parts',
383+
async () => {
384+
const prefix = 'https://links.controls.test/'
385+
const cases = [
386+
{
387+
name: 'A canonical 2048-character destination remains usable',
388+
input: `${prefix}${'a'.repeat(2048 - prefix.length)}`,
389+
expected: `${prefix}${'a'.repeat(2048 - prefix.length)}`,
390+
},
391+
{
392+
name: 'A canonical 2049-character destination does not open an unsavable draft',
393+
input: `${prefix}${'a'.repeat(2049 - prefix.length)}`,
394+
expected: null,
395+
},
396+
{
397+
name: 'URL serialization cannot expand a short Unicode input beyond the saved limit',
398+
input: `${prefix}${'é'.repeat(400)}`,
399+
expected: null,
400+
},
401+
{
402+
name: 'A removable long private query does not reject a short destination',
403+
input: `${prefix}safe?token=${'s'.repeat(3000)}#private-section`,
404+
expected: `${prefix}safe`,
405+
},
406+
]
407+
const failures: string[] = []
408+
for (const entry of cases) {
409+
await page.bringToFront()
410+
const opened = await worker.evaluate(async (linkUrl) => {
411+
const before = await chrome.tabs.query({})
412+
const [tab] = await chrome.tabs.query({ active: true, currentWindow: true })
413+
if (!tab) throw new Error('Missing context-menu source tab')
414+
const clicked = chrome.contextMenus.onClicked as typeof chrome.contextMenus.onClicked & {
415+
dispatch: (info: chrome.contextMenus.OnClickData, tab: chrome.tabs.Tab) => void
416+
}
417+
clicked.dispatch({ menuItemId: 'create-sim-link', linkUrl, editable: false }, tab)
418+
const created = (await chrome.tabs.query({})).filter(
419+
(candidate) => !before.some((existing) => existing.id === candidate.id)
420+
)
421+
const urls = created.map((candidate) => candidate.pendingUrl || candidate.url || '')
422+
for (const candidate of created) {
423+
if (candidate.id !== undefined) await chrome.tabs.remove(candidate.id)
424+
}
425+
return urls
426+
}, entry.input)
427+
if (entry.expected === null) {
428+
if (opened.length !== 0) failures.push(`${entry.name}: opened ${opened.length} draft`)
429+
continue
430+
}
431+
if (opened.length !== 1) {
432+
failures.push(`${entry.name}: opened ${opened.length} drafts`)
433+
continue
434+
}
435+
const draft = new URL(opened[0])
436+
if (
437+
draft.origin !== origin ||
438+
draft.pathname !== '/extension/create-link' ||
439+
draft.search !== '' ||
440+
new URLSearchParams(draft.hash.slice(1)).get('url') !== entry.expected
441+
)
442+
failures.push(`${entry.name}: incorrect private-safe destination handoff`)
443+
}
444+
assert(failures.length === 0, failures.join('\n'))
445+
await page.bringToFront()
446+
}
447+
)
448+
381449
await check('Capture stays local until the trusted hosted frame is ready', async () => {
382450
await capture()
383451
await panel.getByRole('status').filter({ hasText: 'Waiting for Sim' }).waitFor()

‎apps/sim/lib/api/contracts/sim-links.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {
22
SIM_LINK_SUGGESTIONS_PATH,
3+
SIM_LINK_URL_MAX_LENGTH,
34
simLinkSuggestionsSchema,
45
} from '@sim/browser-extension-protocol'
56
import { z } from 'zod'
@@ -44,7 +45,7 @@ export const listSimLinksQuerySchema = z
4445
.object({
4546
search: z.string().max(200).optional(),
4647
/** Only links whose destination is exactly this URL, such as one shown in a search result. */
47-
url: z.string().min(1).max(2048).optional(),
48+
url: z.string().min(1).max(SIM_LINK_URL_MAX_LENGTH).optional(),
4849
cursor: z.string().min(1).max(128).optional(),
4950
sort: z.enum(SIM_LINK_SORTS).optional(),
5051
dir: z.enum(SORT_DIRECTIONS).optional(),

‎apps/sim/lib/sim-links/validation.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
import { SIM_LINK_NAME_MAX_LENGTH } from '@sim/browser-extension-protocol'
1+
import { SIM_LINK_NAME_MAX_LENGTH, SIM_LINK_URL_MAX_LENGTH } from '@sim/browser-extension-protocol'
22
import { z } from 'zod'
33

44
const NAME_PATTERN = /^[A-Za-z0-9.:@&~_-]+$/
55
const NAME_PUNCTUATION = /[.:@&~_-]/g
66
const NAME_DISALLOWED = /[^A-Za-z0-9.:@&~_-]/g
77
export const MAX_MANAGED_BOOKMARKS = 100
88
export const SIM_LINK_DESCRIPTION_MAX_LENGTH = 500
9-
const URL_MAX_LENGTH = 2048
109

1110
/** List orders: alphabetical by name, or most visited first. */
1211
export const SIM_LINK_SORTS = ['name', 'popular'] as const
@@ -120,7 +119,7 @@ const destinationSchema = z
120119
.string()
121120
.trim()
122121
.min(1, 'Enter a destination URL')
123-
.max(URL_MAX_LENGTH, `Use at most ${URL_MAX_LENGTH} characters`)
122+
.max(SIM_LINK_URL_MAX_LENGTH, `Use at most ${SIM_LINK_URL_MAX_LENGTH} characters`)
124123

125124
const simLinkUrlSchema = destinationSchema.refine(
126125
(url) => simLinkOrigin(url) !== null,

‎apps/sim/scripts/test-browser-extension-e2e.ts‎

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ async function seed() {
234234
await tx`insert into workspace (id, project_id, name, owner_id, billed_account_user_id) values (${id}, ${alphaId}, ${`Extension QA workspace ${index + 1}`}, ${owner}, ${owner})`
235235
await tx`insert into permissions (id, user_id, entity_type, entity_id, permission_type) values (${generateId()}, ${owner}, 'workspace', ${id}, 'admin')`
236236
}
237+
await tx`update workspace set created_at = '2020-01-01T00:00:00Z' where id = ${alphaId}`
237238
await tx`insert into workflow (id, user_id, workspace_id, name, last_synced, created_at, updated_at) values (${workflowId}, ${owner}, ${alphaId}, 'Extension QA workflow', now(), now(), now())`
238239
await tx`insert into "user" (id, name, email, email_verified, created_at, updated_at) values (${otherUserId}, 'Other extension QA user', ${`extension-other-${otherUserId}@example.test`}, false, now(), now())`
239240
await tx`insert into project (id, name, owner_id) values (${deniedId}, 'Inaccessible QA project', ${otherUserId})`
@@ -340,8 +341,26 @@ try {
340341
context.setDefaultNavigationTimeout(120_000)
341342
let worker = context.serviceWorkers()[0] ?? (await context.waitForEvent('serviceworker'))
342343
const extensionId = worker.url().split('/')[2]
343-
const openPanel = async () => {
344+
const setPageViewport = async (page: Page, width: number, height: number) => {
345+
await page.setViewportSize({ width, height })
346+
const session = await browserContext.newCDPSession(page)
347+
try {
348+
const { windowId } = await session.send('Browser.getWindowForTarget')
349+
// OOPIF hit testing uses the native content bounds, including below the browser toolbar.
350+
await session.send('Browser.setContentsSize', { windowId, width, height })
351+
} finally {
352+
await session.detach()
353+
}
354+
}
355+
const openPage = async () => {
344356
const page = await browserContext.newPage()
357+
const viewport = page.viewportSize()
358+
assert(viewport, 'Browser fixture must declare its viewport')
359+
await setPageViewport(page, viewport.width, viewport.height)
360+
return page
361+
}
362+
const openPanel = async () => {
363+
const page = await openPage()
345364
await page.route('**/api/settings/voice', (route) =>
346365
route.fulfill({ json: { sttAvailable: true } })
347366
)
@@ -393,17 +412,7 @@ try {
393412
}
394413
panel = await openPanel()
395414
let shell = panel
396-
const setPanelViewport = async (width: number, height: number) => {
397-
await shell.setViewportSize({ width, height })
398-
const session = await browserContext.newCDPSession(shell)
399-
try {
400-
const { windowId } = await session.send('Browser.getWindowForTarget')
401-
// OOPIF hit testing uses the native content bounds, including below the browser toolbar.
402-
await session.send('Browser.setContentsSize', { windowId, width, height })
403-
} finally {
404-
await session.detach()
405-
}
406-
}
415+
const setPanelViewport = (width: number, height: number) => setPageViewport(shell, width, height)
407416
await panel.goto(`chrome-extension://${extensionId}/sidepanel.html`)
408417
const frame = () => {
409418
const app = shell.frames().find((candidate) => candidate.url().startsWith(base.origin))
@@ -469,7 +478,7 @@ try {
469478
await navigate(`${base.origin}/extension/${alphaId}`)
470479
await expect(frame().getByRole('textbox', { name: 'Message', exact: true })).toBeVisible()
471480
}
472-
let source = await context.newPage()
481+
let source = await openPage()
473482
await source.goto(`${fixtureOrigin}/page?private=QUERY_SECRET#FRAGMENT_SECRET`)
474483
const observeCaptureReceipts = () =>
475484
shell.evaluate((origin) => {
@@ -796,7 +805,7 @@ try {
796805
await checkComposer(
797806
'The ordinary web composer accepts text selections and rejects links and remote images',
798807
async () => {
799-
const app = await browserContext.newPage()
808+
const app = await openPage()
800809
const remote = new URL('/ordinary-drop-image.png', fixtureOrigin)
801810
const navigationAttempts: string[] = []
802811
await browserContext.route(remote.href, (route) => {
@@ -1512,12 +1521,12 @@ try {
15121521
async () => {
15131522
const endpoint = `${base.origin}/api/mothership/chat`
15141523
const received: Record<string, unknown>[] = []
1515-
const second = await browserContext.newPage()
1524+
const second = await openPage()
15161525
await second.goto(`${fixtureOrigin}/reference-second?private=DO_NOT_SEND#private`)
15171526
await second.locator('article p').evaluate((el) => {
15181527
el.textContent = 'Second active tab: JADE-231.'
15191528
})
1520-
const restricted = await browserContext.newPage()
1529+
const restricted = await openPage()
15211530
await restricted.goto(`http://ungranted.localhost:${fixtureAddress.port}/denied`)
15221531
await browserContext.route(endpoint, (route) => {
15231532
if (route.request().method() !== 'POST') return route.continue()
@@ -1973,7 +1982,7 @@ try {
19731982
return animation
19741983
}
19751984
})
1976-
source = await browserContext.newPage()
1985+
source = await openPage()
19771986
await source.goto(`${fixtureOrigin}/page?private=QUERY_SECRET#FRAGMENT_SECRET`)
19781987
await shell.goto(`chrome-extension://${extensionId}/sidepanel.html`)
19791988
await observeCaptureReceipts()
@@ -2254,6 +2263,10 @@ try {
22542263
'A transient workspace-list refetch unmounted the picker'
22552264
)
22562265
assert.equal(await scroller.evaluate((element) => element.scrollTop), scrollTop)
2266+
await expect(choices.getByRole('link').last()).toHaveAttribute(
2267+
'href',
2268+
`/extension/${alphaId}`
2269+
)
22572270
await screenshot('workspace-picker-refetch-outage')
22582271
await choices.getByRole('link', { name: 'Extension QA alpha', exact: true }).click()
22592272
await expect(frame().getByRole('textbox', { name: 'Message', exact: true })).toBeVisible()

‎packages/browser-extension-protocol/src/index.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,9 @@ export type ExtensionToAppMessage = z.infer<typeof extensionToAppMessageSchema>
120120
/** The longest Sim link name; the extension asks about nothing longer. */
121121
export const SIM_LINK_NAME_MAX_LENGTH = 64
122122

123+
/** Destination limit shared by editable drafts and saved Sim links. */
124+
export const SIM_LINK_URL_MAX_LENGTH = 2048
125+
123126
/** Where the extension asks for the address-bar suggestions behind its `sim` keyword. */
124127
export const SIM_LINK_SUGGESTIONS_PATH = '/api/sim-links/suggestions'
125128

@@ -143,7 +146,7 @@ export function simLinkDraftDestination(value: string): string | null {
143146
if (!['http:', 'https:'].includes(url.protocol) || url.username || url.password) return null
144147
url.search = ''
145148
url.hash = ''
146-
return url.href
149+
return url.href.length <= SIM_LINK_URL_MAX_LENGTH ? url.href : null
147150
} catch {
148151
return null
149152
}

0 commit comments

Comments
 (0)