Skip to content

Commit f54148d

Browse files
knightedcodemonkeyknightedcodemonkey
committed
refactor: address pr comments.
1 parent 107e34d commit f54148d

7 files changed

Lines changed: 219 additions & 101 deletions

File tree

‎.github/instructions/pr-review.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ You are reviewing changes for @knighted/develop. Be concise, technical, and spec
2323

2424
## What to verify
2525

26+
- No changes reintroduce cross-workspace overwrite/delete behavior.
2627
- No generated artifacts are edited (dist/, coverage/, test-results/).
2728
- Duplicated logic paths are avoided when a shared helper/module already exists; prefer reusing the established implementation.
2829
- CDN import/fallback behavior is not bypassed with ad hoc URLs in feature modules.

‎src/app.js‎

Lines changed: 9 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,7 @@ import { createPrContextStateChangeHandler } from './modules/app-core/pr-context
5252
import { createWorkspaceContextStatusController } from './modules/app-core/workspace-context-status-controller.js'
5353
import { createWorkspaceRecordAppliedHandler } from './modules/app-core/workspace-record-applied-handler.js'
5454
import { createGitHubChatWorkspaceActions } from './modules/app-core/github-chat-workspace-actions.js'
55-
import {
56-
encodeWorkspaceSharePayload,
57-
isNativeWorkspaceShareCodecSupported,
58-
workspaceShareParam,
59-
} from './modules/app-core/workspace-share-codec.js'
55+
import { createShareCurrentLocalWorkspace } from './modules/app-core/workspace-share-action.js'
6056
import { createDiagnosticsUiController } from './modules/diagnostics/diagnostics-ui.js'
6157
import { createGitHubChatDrawer } from './modules/github/chat/drawer.js'
6258
import { createGitHubByotControls } from './modules/github/byot-controls.js'
@@ -991,40 +987,14 @@ const { syncActiveWorkspaceRepositoryScope, forkWorkspaceFromCurrentState } =
991987
},
992988
})
993989

994-
const maxWorkspaceShareUrlLength = 8000
995-
996-
const shareCurrentLocalWorkspace = async () => {
997-
if (!clipboardSupported) {
998-
throw new Error('Clipboard API is not available in this browser context.')
999-
}
1000-
1001-
if (!isNativeWorkspaceShareCodecSupported()) {
1002-
throw new Error('Native compression is not supported in this browser context.')
1003-
}
1004-
1005-
if (workspaceScopeMarker !== 'local') {
1006-
throw new Error('Share is only available for local workspaces.')
1007-
}
1008-
1009-
await flushWorkspaceSave({ preserveRecordId: true })
1010-
const snapshot = buildWorkspaceRecordSnapshot()
1011-
if (!snapshot || typeof snapshot !== 'object') {
1012-
throw new Error('Could not prepare workspace snapshot.')
1013-
}
1014-
1015-
const encodedPayload = await encodeWorkspaceSharePayload(snapshot)
1016-
const sharedUrl = new URL(window.location.href)
1017-
sharedUrl.searchParams.set(workspaceShareParam, encodedPayload)
1018-
const sharedUrlText = sharedUrl.toString()
1019-
1020-
if (sharedUrlText.length > maxWorkspaceShareUrlLength) {
1021-
throw new Error('Workspace is too large for a URL.')
1022-
}
1023-
1024-
await navigator.clipboard.writeText(sharedUrlText)
1025-
setStatus('Share link copied', 'neutral')
1026-
showAppToast('Share link copied to clipboard.')
1027-
}
990+
const shareCurrentLocalWorkspace = createShareCurrentLocalWorkspace({
991+
clipboardSupported,
992+
getWorkspaceScopeMarker: () => workspaceScopeMarker,
993+
flushWorkspaceSave,
994+
buildWorkspaceRecordSnapshot,
995+
setStatus,
996+
showAppToast,
997+
})
1028998

1029999
editedIndicatorVisibilityController.setRefreshHandlers({
10301000
syncHeaderLabels,

‎src/modules/app-core/app-bindings-startup.js‎

Lines changed: 8 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
import {
2-
decodeWorkspaceSharePayload,
3-
workspaceShareParam,
4-
} from './workspace-share-codec.js'
1+
import { createWorkspaceShareUrlImporter } from './workspace-share-url-import.js'
52

63
const bindAppEventsAndStart = ({
74
editorUi,
@@ -90,6 +87,7 @@ const bindAppEventsAndStart = ({
9087
getPrimaryStyleWorkspaceTab,
9188
workspaceSaveController,
9289
workspaceStorage,
90+
createWorkspaceRecordId,
9391
syncDiagnosticsDrawerLayout,
9492
setHasCompletedInitialWorkspaceBootstrap,
9593
} = workspaceUi
@@ -125,61 +123,12 @@ const bindAppEventsAndStart = ({
125123
initializeCodeEditors,
126124
} = startup
127125

128-
const toShareableWorkspaceRecord = snapshot => {
129-
if (!snapshot || typeof snapshot !== 'object') {
130-
return null
131-
}
132-
133-
const nextTabs = Array.isArray(snapshot.tabs) ? snapshot.tabs : []
134-
if (nextTabs.length === 0) {
135-
return null
136-
}
137-
138-
return {
139-
...snapshot,
140-
id: typeof snapshot.id === 'string' && snapshot.id.trim() ? snapshot.id.trim() : '',
141-
workspaceScope: 'local',
142-
repo: '',
143-
base: '',
144-
head: '',
145-
prNumber: null,
146-
prTitle: '',
147-
prContextState: 'inactive',
148-
workspaceKey: '',
149-
lastModified: Date.now(),
150-
createdAt: Date.now(),
151-
}
152-
}
153-
154-
const clearWorkspaceShareParamFromUrl = () => {
155-
const currentUrl = new URL(window.location.href)
156-
currentUrl.searchParams.delete(workspaceShareParam)
157-
window.history.replaceState(window.history.state, '', currentUrl.toString())
158-
}
159-
160-
const importWorkspaceFromShareUrl = async () => {
161-
const currentUrl = new URL(window.location.href)
162-
const encodedPayload = currentUrl.searchParams.get(workspaceShareParam)
163-
if (!encodedPayload) {
164-
return false
165-
}
166-
167-
const decodedSnapshot = await decodeWorkspaceSharePayload(encodedPayload)
168-
const importedRecord = toShareableWorkspaceRecord(decodedSnapshot)
169-
if (!importedRecord) {
170-
throw new Error('Shared workspace payload is missing a valid tab snapshot.')
171-
}
172-
173-
const savedWorkspace = await workspaceStorage.upsertWorkspace(importedRecord)
174-
const didApply = await applyWorkspaceRecord(savedWorkspace, { silent: false })
175-
176-
if (didApply) {
177-
clearWorkspaceShareParamFromUrl()
178-
await refreshLocalContextOptions()
179-
}
180-
181-
return didApply
182-
}
126+
const importWorkspaceFromShareUrl = createWorkspaceShareUrlImporter({
127+
workspaceStorage,
128+
applyWorkspaceRecord,
129+
refreshLocalContextOptions,
130+
createWorkspaceRecordId,
131+
})
183132
const clearComponentSource = () => {
184133
setJsxSource('')
185134
clearDiagnosticsScope('component')

‎src/modules/app-core/github-workflows.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,6 @@ const initializeGitHubWorkflows = ({
482482

483483
try {
484484
await shareCurrentLocalWorkspace()
485-
workspacesDrawerController?.setStatus('Share link copied.', 'neutral')
486485
return true
487486
} catch (error) {
488487
const message =

‎src/modules/app-core/workspace-share-action.js‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import {
2+
encodeWorkspaceSharePayload,
3+
isNativeWorkspaceShareCodecSupported,
4+
workspaceShareParam,
5+
} from './workspace-share-codec.js'
6+
7+
const defaultMaxWorkspaceShareUrlLength = 8000
8+
9+
const createShareCurrentLocalWorkspace = ({
10+
clipboardSupported,
11+
getWorkspaceScopeMarker,
12+
flushWorkspaceSave,
13+
buildWorkspaceRecordSnapshot,
14+
setStatus,
15+
showAppToast,
16+
maxWorkspaceShareUrlLength = defaultMaxWorkspaceShareUrlLength,
17+
} = {}) => {
18+
return async () => {
19+
if (!clipboardSupported) {
20+
throw new Error('Clipboard API is not available in this browser context.')
21+
}
22+
23+
if (!isNativeWorkspaceShareCodecSupported()) {
24+
throw new Error('Native compression is not supported in this browser context.')
25+
}
26+
27+
if (getWorkspaceScopeMarker?.() !== 'local') {
28+
throw new Error('Share is only available for local workspaces.')
29+
}
30+
31+
await flushWorkspaceSave({ preserveRecordId: true })
32+
const snapshot = buildWorkspaceRecordSnapshot()
33+
if (!snapshot || typeof snapshot !== 'object') {
34+
throw new Error('Could not prepare workspace snapshot.')
35+
}
36+
37+
const encodedPayload = await encodeWorkspaceSharePayload(snapshot)
38+
const sharedUrl = new URL(window.location.href)
39+
sharedUrl.searchParams.set(workspaceShareParam, encodedPayload)
40+
const sharedUrlText = sharedUrl.toString()
41+
42+
if (sharedUrlText.length > maxWorkspaceShareUrlLength) {
43+
throw new Error('Workspace is too large for a URL.')
44+
}
45+
46+
await navigator.clipboard.writeText(sharedUrlText)
47+
setStatus('Share link copied', 'neutral')
48+
showAppToast('Share link copied to clipboard.')
49+
}
50+
}
51+
52+
export { createShareCurrentLocalWorkspace }

‎src/modules/app-core/workspace-share-codec.js‎

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
const workspaceShareParam = 'sws'
22
const workspaceShareSchemaVersion = 1
33
const workspaceShareCompression = 'gzip'
4+
const maxWorkspaceShareEncodedPayloadLength = 8192
5+
const maxWorkspaceShareDecodedBytes = 1024 * 1024
6+
const maxWorkspaceShareExpansionRatio = 100
47

58
const isNativeWorkspaceShareCodecSupported = () => {
69
return (
@@ -53,6 +56,60 @@ const streamToUint8Array = async stream => {
5356
return new Uint8Array(buffer)
5457
}
5558

59+
const streamToUint8ArrayWithLimit = async ({
60+
stream,
61+
maxBytes,
62+
compressedBytesLength,
63+
maxExpansionRatio,
64+
}) => {
65+
const reader = stream.getReader()
66+
const chunks = []
67+
let totalBytes = 0
68+
69+
try {
70+
while (true) {
71+
// Sequential reads are required for Web Streams reader consumption.
72+
// eslint-disable-next-line no-await-in-loop
73+
const { done, value } = await reader.read()
74+
if (done) {
75+
break
76+
}
77+
78+
if (!(value instanceof Uint8Array)) {
79+
continue
80+
}
81+
82+
totalBytes += value.byteLength
83+
if (totalBytes > maxBytes) {
84+
throw new Error('Workspace share payload exceeds maximum decoded size.')
85+
}
86+
87+
if (
88+
typeof compressedBytesLength === 'number' &&
89+
compressedBytesLength > 0 &&
90+
typeof maxExpansionRatio === 'number' &&
91+
maxExpansionRatio > 0 &&
92+
totalBytes > compressedBytesLength * maxExpansionRatio
93+
) {
94+
throw new Error('Workspace share payload expansion ratio is too large.')
95+
}
96+
97+
chunks.push(value)
98+
}
99+
} finally {
100+
reader.releaseLock()
101+
}
102+
103+
const bytes = new Uint8Array(totalBytes)
104+
let offset = 0
105+
for (const chunk of chunks) {
106+
bytes.set(chunk, offset)
107+
offset += chunk.byteLength
108+
}
109+
110+
return bytes
111+
}
112+
56113
const compressText = async text => {
57114
const encoder = new TextEncoder()
58115
const sourceBytes = encoder.encode(text)
@@ -69,7 +126,12 @@ const decompressText = async bytes => {
69126
const decompressedStream = sourceStream.pipeThrough(
70127
new DecompressionStream(workspaceShareCompression),
71128
)
72-
const decompressedBytes = await streamToUint8Array(decompressedStream)
129+
const decompressedBytes = await streamToUint8ArrayWithLimit({
130+
stream: decompressedStream,
131+
maxBytes: maxWorkspaceShareDecodedBytes,
132+
compressedBytesLength: bytes?.byteLength ?? 0,
133+
maxExpansionRatio: maxWorkspaceShareExpansionRatio,
134+
})
73135
const decoder = new TextDecoder()
74136
return decoder.decode(decompressedBytes)
75137
}
@@ -92,7 +154,12 @@ const encodeWorkspaceSharePayload = async snapshot => {
92154

93155
const serialized = JSON.stringify(envelope)
94156
const compressed = await compressText(serialized)
95-
return toBase64Url(compressed)
157+
const encoded = toBase64Url(compressed)
158+
if (encoded.length > maxWorkspaceShareEncodedPayloadLength) {
159+
throw new Error('Workspace share payload is too large.')
160+
}
161+
162+
return encoded
96163
}
97164

98165
const decodeWorkspaceSharePayload = async encodedPayload => {
@@ -104,6 +171,10 @@ const decodeWorkspaceSharePayload = async encodedPayload => {
104171
throw new TypeError('Workspace share payload must be a non-empty string.')
105172
}
106173

174+
if (encodedPayload.trim().length > maxWorkspaceShareEncodedPayloadLength) {
175+
throw new Error('Workspace share payload exceeds maximum encoded length.')
176+
}
177+
107178
let parsed = null
108179
try {
109180
const compressedBytes = fromBase64Url(encodedPayload.trim())

0 commit comments

Comments
 (0)