mirror of
https://github.com/stablyai/orca.git
synced 2026-10-02 16:02:15 +00:00
sim: merge PR #17264 (hand-resolved)
This commit is contained in:
@@ -86,7 +86,8 @@ describe('GitHub IPC channel parity', () => {
|
||||
github.registerGitHubHandlers(harness.store as never, harness.stats as never)
|
||||
|
||||
const preloadSource = readFileSync(new URL('../../preload/index.ts', import.meta.url), 'utf8')
|
||||
const exposedChannels = [...preloadSource.matchAll(/ipcRenderer\.invoke\('(gh:[^']+)'/g)].map(
|
||||
// Channels go through the preload boundary wrapper, so match the call rather than the receiver.
|
||||
const exposedChannels = [...preloadSource.matchAll(/(?<![\w.])invoke\('(gh:[^']+)'/g)].map(
|
||||
(match) => match[1]
|
||||
)
|
||||
const registeredChannels = mocks.electron.ipcMain.handle.mock.calls.map(
|
||||
|
||||
+29
-35
@@ -2,7 +2,7 @@
|
||||
adding or changing a `gl.*` channel doesn't surface as a merge
|
||||
conflict on every upstream sync of the much larger central preload
|
||||
file. Composed back into `api.gl` from `index.ts`. */
|
||||
import { ipcRenderer } from 'electron'
|
||||
import { invoke } from './ipc-invoke-boundary'
|
||||
import type { TaskSourceContext } from '../shared/task-source-context'
|
||||
|
||||
type GitLabRepoSelectorArgs = {
|
||||
@@ -12,23 +12,23 @@ type GitLabRepoSelectorArgs = {
|
||||
}
|
||||
|
||||
export const glApi = {
|
||||
viewer: (): Promise<unknown> => ipcRenderer.invoke('gitlab:viewer'),
|
||||
diagnoseAuth: (): Promise<unknown> => ipcRenderer.invoke('gitlab:diagnoseAuth'),
|
||||
viewer: (): Promise<unknown> => invoke('gitlab:viewer'),
|
||||
diagnoseAuth: (): Promise<unknown> => invoke('gitlab:diagnoseAuth'),
|
||||
rateLimit: (args?: { force?: boolean; host?: string | null }): Promise<unknown> =>
|
||||
ipcRenderer.invoke('gitlab:rateLimit', args),
|
||||
invoke('gitlab:rateLimit', args),
|
||||
|
||||
projectSlug: (args: GitLabRepoSelectorArgs): Promise<unknown> =>
|
||||
ipcRenderer.invoke('gitlab:projectSlug', args),
|
||||
invoke('gitlab:projectSlug', args),
|
||||
|
||||
mrForBranch: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
branch: string
|
||||
linkedMRIid?: number | null
|
||||
}
|
||||
): Promise<unknown> => ipcRenderer.invoke('gitlab:mrForBranch', args),
|
||||
): Promise<unknown> => invoke('gitlab:mrForBranch', args),
|
||||
|
||||
mr: (args: GitLabRepoSelectorArgs & { iid: number }): Promise<unknown> =>
|
||||
ipcRenderer.invoke('gitlab:mr', args),
|
||||
invoke('gitlab:mr', args),
|
||||
|
||||
listMRs: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
@@ -37,7 +37,7 @@ export const glApi = {
|
||||
perPage?: number
|
||||
query?: string
|
||||
}
|
||||
): Promise<unknown> => ipcRenderer.invoke('gitlab:listMRs', args),
|
||||
): Promise<unknown> => invoke('gitlab:listMRs', args),
|
||||
|
||||
listWorkItems: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
@@ -46,10 +46,10 @@ export const glApi = {
|
||||
perPage?: number
|
||||
query?: string
|
||||
}
|
||||
): Promise<unknown> => ipcRenderer.invoke('gitlab:listWorkItems', args),
|
||||
): Promise<unknown> => invoke('gitlab:listWorkItems', args),
|
||||
|
||||
issue: (args: GitLabRepoSelectorArgs & { number: number }): Promise<unknown> =>
|
||||
ipcRenderer.invoke('gitlab:issue', args),
|
||||
invoke('gitlab:issue', args),
|
||||
|
||||
listIssues: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
@@ -59,7 +59,7 @@ export const glApi = {
|
||||
page?: number
|
||||
}
|
||||
): Promise<{ items: unknown[]; totalPages?: number; error?: unknown }> =>
|
||||
ipcRenderer.invoke('gitlab:listIssues', args),
|
||||
invoke('gitlab:listIssues', args),
|
||||
|
||||
createIssue: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
@@ -67,65 +67,59 @@ export const glApi = {
|
||||
body: string
|
||||
}
|
||||
): Promise<{ ok: true; number: number; url: string } | { ok: false; error: string }> =>
|
||||
ipcRenderer.invoke('gitlab:createIssue', args),
|
||||
invoke('gitlab:createIssue', args),
|
||||
|
||||
updateIssue: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
number: number
|
||||
updates: unknown
|
||||
}
|
||||
): Promise<{ ok: true } | { ok: false; error: string }> =>
|
||||
ipcRenderer.invoke('gitlab:updateIssue', args),
|
||||
): Promise<{ ok: true } | { ok: false; error: string }> => invoke('gitlab:updateIssue', args),
|
||||
|
||||
addIssueComment: (
|
||||
args: GitLabRepoSelectorArgs & { number: number; body: string }
|
||||
): Promise<unknown> => ipcRenderer.invoke('gitlab:addIssueComment', args),
|
||||
): Promise<unknown> => invoke('gitlab:addIssueComment', args),
|
||||
|
||||
listLabels: (args: GitLabRepoSelectorArgs): Promise<string[]> =>
|
||||
ipcRenderer.invoke('gitlab:listLabels', args),
|
||||
invoke('gitlab:listLabels', args),
|
||||
|
||||
listAssignableUsers: (args: GitLabRepoSelectorArgs): Promise<unknown[]> =>
|
||||
ipcRenderer.invoke('gitlab:listAssignableUsers', args),
|
||||
invoke('gitlab:listAssignableUsers', args),
|
||||
|
||||
todos: (args: GitLabRepoSelectorArgs): Promise<unknown[]> =>
|
||||
ipcRenderer.invoke('gitlab:todos', args),
|
||||
todos: (args: GitLabRepoSelectorArgs): Promise<unknown[]> => invoke('gitlab:todos', args),
|
||||
|
||||
workItemDetails: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
iid: number
|
||||
type: 'issue' | 'mr'
|
||||
}
|
||||
): Promise<unknown> => ipcRenderer.invoke('gitlab:workItemDetails', args),
|
||||
): Promise<unknown> => invoke('gitlab:workItemDetails', args),
|
||||
|
||||
closeMR: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
iid: number
|
||||
}
|
||||
): Promise<{ ok: true } | { ok: false; error: string }> =>
|
||||
ipcRenderer.invoke('gitlab:closeMR', args),
|
||||
): Promise<{ ok: true } | { ok: false; error: string }> => invoke('gitlab:closeMR', args),
|
||||
|
||||
reopenMR: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
iid: number
|
||||
}
|
||||
): Promise<{ ok: true } | { ok: false; error: string }> =>
|
||||
ipcRenderer.invoke('gitlab:reopenMR', args),
|
||||
): Promise<{ ok: true } | { ok: false; error: string }> => invoke('gitlab:reopenMR', args),
|
||||
|
||||
mergeMR: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
iid: number
|
||||
method?: 'merge' | 'squash' | 'rebase'
|
||||
}
|
||||
): Promise<{ ok: true } | { ok: false; error: string }> =>
|
||||
ipcRenderer.invoke('gitlab:mergeMR', args),
|
||||
): Promise<{ ok: true } | { ok: false; error: string }> => invoke('gitlab:mergeMR', args),
|
||||
|
||||
updateMR: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
iid: number
|
||||
updates: unknown
|
||||
}
|
||||
): Promise<{ ok: true } | { ok: false; error: string }> =>
|
||||
ipcRenderer.invoke('gitlab:updateMR', args),
|
||||
): Promise<{ ok: true } | { ok: false; error: string }> => invoke('gitlab:updateMR', args),
|
||||
|
||||
updateMRReviewers: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
@@ -133,10 +127,10 @@ export const glApi = {
|
||||
reviewerIds: number[]
|
||||
projectRef?: unknown
|
||||
}
|
||||
): Promise<unknown> => ipcRenderer.invoke('gitlab:updateMRReviewers', args),
|
||||
): Promise<unknown> => invoke('gitlab:updateMRReviewers', args),
|
||||
|
||||
addMRComment: (args: GitLabRepoSelectorArgs & { iid: number; body: string }): Promise<unknown> =>
|
||||
ipcRenderer.invoke('gitlab:addMRComment', args),
|
||||
invoke('gitlab:addMRComment', args),
|
||||
|
||||
addMRInlineComment: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
@@ -144,7 +138,7 @@ export const glApi = {
|
||||
input: unknown
|
||||
projectRef?: unknown
|
||||
}
|
||||
): Promise<unknown> => ipcRenderer.invoke('gitlab:addMRInlineComment', args),
|
||||
): Promise<unknown> => invoke('gitlab:addMRInlineComment', args),
|
||||
|
||||
resolveMRDiscussion: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
@@ -152,15 +146,15 @@ export const glApi = {
|
||||
discussionId: string
|
||||
resolved: boolean
|
||||
}
|
||||
): Promise<unknown> => ipcRenderer.invoke('gitlab:resolveMRDiscussion', args),
|
||||
): Promise<unknown> => invoke('gitlab:resolveMRDiscussion', args),
|
||||
|
||||
jobTrace: (
|
||||
args: GitLabRepoSelectorArgs & { jobId: number; projectRef?: unknown; logExcerpt?: boolean }
|
||||
): Promise<unknown> => ipcRenderer.invoke('gitlab:jobTrace', args),
|
||||
): Promise<unknown> => invoke('gitlab:jobTrace', args),
|
||||
|
||||
retryJob: (
|
||||
args: GitLabRepoSelectorArgs & { jobId: number; projectRef?: unknown }
|
||||
): Promise<unknown> => ipcRenderer.invoke('gitlab:retryJob', args),
|
||||
): Promise<unknown> => invoke('gitlab:retryJob', args),
|
||||
|
||||
workItemByPath: (
|
||||
args: GitLabRepoSelectorArgs & {
|
||||
@@ -169,5 +163,5 @@ export const glApi = {
|
||||
iid: number
|
||||
type: 'issue' | 'mr'
|
||||
}
|
||||
): Promise<unknown> => ipcRenderer.invoke('gitlab:workItemByPath', args)
|
||||
): Promise<unknown> => invoke('gitlab:workItemByPath', args)
|
||||
}
|
||||
|
||||
+717
-813
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,281 @@
|
||||
import { spawnSync } from 'node:child_process'
|
||||
import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { createRequire } from 'node:module'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterAll, describe, expect, it } from 'vitest'
|
||||
import { build as buildVite } from 'vite'
|
||||
import { resolveElectronProbeLaunch } from '../main/browser/electron-probe-display-launch'
|
||||
|
||||
/**
|
||||
* What a renderer consumer actually receives, measured across a real `contextBridge`.
|
||||
*
|
||||
* The boundary rethrows the object `ipcRenderer.invoke` rejected with, so inside preload the
|
||||
* rejection keeps its identity, its properties and its original stack. None of that reaches the
|
||||
* renderer: `contextBridge` copies the value, and a copy is a fresh plain `Error`. Unit tests that
|
||||
* stop at the preload side cannot see this, which is why the guarantee was overstated — so this
|
||||
* file drives the real binary and asserts the renderer's view, including the parts that are lost.
|
||||
*
|
||||
* The identity and own-property assertions describe the bridge, not the strip, and would hold with
|
||||
* the boundary deleted. They are here to keep the *claim* honest, not to pin the fix; the strip is
|
||||
* pinned by the message and stack assertions, which the `unstripped` control moves.
|
||||
*/
|
||||
const electronBinary = createRequire(import.meta.url)('electron') as string
|
||||
const fixtureRoots: string[] = []
|
||||
|
||||
type ErrorView = {
|
||||
isError: boolean
|
||||
ctorName: string
|
||||
name: string
|
||||
message: string
|
||||
ownKeys: string[]
|
||||
stackFirstLine: string
|
||||
code?: string
|
||||
}
|
||||
|
||||
type FixtureResult = {
|
||||
preloadStripped: ErrorView
|
||||
preloadCarriesOwnProperties: ErrorView
|
||||
rendererStripped: ErrorView
|
||||
rendererUnstripped: ErrorView
|
||||
rendererCarriesOwnProperties: ErrorView
|
||||
sameObjectAcrossTwoRejections: boolean
|
||||
mutatingOneCopyLeaksToTheOther: boolean
|
||||
}
|
||||
|
||||
afterAll(() => {
|
||||
for (const root of fixtureRoots) {
|
||||
rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 })
|
||||
}
|
||||
})
|
||||
|
||||
/** `sandbox: true` matches `createMainWindow`, and a sandboxed preload may only require `electron`. */
|
||||
function preloadEntry(boundaryPath: string): string {
|
||||
return `
|
||||
import { contextBridge, ipcRenderer } from 'electron'
|
||||
import { invoke } from ${JSON.stringify(boundaryPath)}
|
||||
|
||||
const view = (e) => ({
|
||||
isError: e instanceof Error,
|
||||
ctorName: (e && e.constructor && e.constructor.name) || '',
|
||||
name: (e && e.name) || '',
|
||||
message: (e && e.message) || '',
|
||||
ownKeys: e && typeof e === 'object' ? Object.getOwnPropertyNames(e).sort() : [],
|
||||
stackFirstLine: e && e.stack ? String(e.stack).split('\\n')[0] : '',
|
||||
code: e && e.code
|
||||
})
|
||||
|
||||
const record = {}
|
||||
|
||||
// The real boundary, on a channel whose handler rejects with a readable reason.
|
||||
const stripped = invoke('probe:reject').catch((rejection) => {
|
||||
record.preloadStripped = view(rejection)
|
||||
throw rejection
|
||||
})
|
||||
|
||||
// Control: the same handler reached without the boundary, so the envelope is still on the message.
|
||||
const unstripped = ipcRenderer.invoke('probe:reject-control')
|
||||
|
||||
// A preload-constructed error carrying own properties, to characterise the bridge itself.
|
||||
class BoundaryProbeError extends Error {
|
||||
constructor(message) {
|
||||
super(message)
|
||||
this.name = 'BoundaryProbeError'
|
||||
this.code = 'E_PROBE'
|
||||
}
|
||||
}
|
||||
const carrier = new BoundaryProbeError('carries own properties')
|
||||
record.preloadCarriesOwnProperties = view(carrier)
|
||||
|
||||
// One object, rejected twice: the renderer sees two copies or one reference.
|
||||
const shared = new Error('rejected twice')
|
||||
|
||||
contextBridge.exposeInMainWorld('probe', {
|
||||
stripped: () => stripped,
|
||||
unstripped: () => unstripped,
|
||||
carrier: () => Promise.reject(carrier),
|
||||
sharedFirst: () => Promise.reject(shared),
|
||||
sharedSecond: () => Promise.reject(shared),
|
||||
record: () => record
|
||||
})
|
||||
`
|
||||
}
|
||||
|
||||
const RENDERER_PROBE = `
|
||||
const view = (e) => ({
|
||||
isError: e instanceof Error,
|
||||
ctorName: (e && e.constructor && e.constructor.name) || '',
|
||||
name: (e && e.name) || '',
|
||||
message: (e && e.message) || '',
|
||||
ownKeys: e && typeof e === 'object' ? Object.getOwnPropertyNames(e).sort() : [],
|
||||
stackFirstLine: e && e.stack ? String(e.stack).split('\\n')[0] : '',
|
||||
code: e && e.code
|
||||
})
|
||||
const rejection = async (call) => {
|
||||
try {
|
||||
await call()
|
||||
} catch (caught) {
|
||||
return caught
|
||||
}
|
||||
throw new Error('expected a rejection')
|
||||
}
|
||||
window.__probe = async () => {
|
||||
const stripped = await rejection(() => window.probe.stripped())
|
||||
const unstripped = await rejection(() => window.probe.unstripped())
|
||||
const carrier = await rejection(() => window.probe.carrier())
|
||||
const first = await rejection(() => window.probe.sharedFirst())
|
||||
const second = await rejection(() => window.probe.sharedSecond())
|
||||
first.message = 'mutated in the renderer'
|
||||
return {
|
||||
...window.probe.record(),
|
||||
rendererStripped: view(stripped),
|
||||
rendererUnstripped: view(unstripped),
|
||||
rendererCarriesOwnProperties: view(carrier),
|
||||
sameObjectAcrossTwoRejections: first === second,
|
||||
mutatingOneCopyLeaksToTheOther: second.message === 'mutated in the renderer'
|
||||
}
|
||||
}
|
||||
`
|
||||
|
||||
function fixtureMain(paths: { htmlPath: string; preloadPath: string; resultPath: string }): string {
|
||||
return `
|
||||
const { app, BrowserWindow, ipcMain } = require('electron')
|
||||
const { writeFileSync } = require('node:fs')
|
||||
|
||||
class HandlerError extends Error {
|
||||
constructor(message) {
|
||||
super(message)
|
||||
this.name = 'HandlerError'
|
||||
this.code = 'E_HANDLER'
|
||||
}
|
||||
}
|
||||
const reject = () => {
|
||||
throw new HandlerError('Host key verification failed')
|
||||
}
|
||||
ipcMain.handle('probe:reject', reject)
|
||||
ipcMain.handle('probe:reject-control', reject)
|
||||
|
||||
const timeout = setTimeout(() => {
|
||||
writeFileSync(${JSON.stringify(paths.resultPath)}, JSON.stringify({ error: 'fixture timeout' }))
|
||||
process.exit(1)
|
||||
}, 30000)
|
||||
|
||||
app.whenReady().then(async () => {
|
||||
try {
|
||||
const window = new BrowserWindow({
|
||||
show: false,
|
||||
webPreferences: {
|
||||
preload: ${JSON.stringify(paths.preloadPath)},
|
||||
sandbox: true,
|
||||
contextIsolation: true,
|
||||
nodeIntegration: false
|
||||
}
|
||||
})
|
||||
await window.loadFile(${JSON.stringify(paths.htmlPath)})
|
||||
const result = await window.webContents.executeJavaScript('window.__probe()')
|
||||
clearTimeout(timeout)
|
||||
writeFileSync(${JSON.stringify(paths.resultPath)}, JSON.stringify(result))
|
||||
app.exit(0)
|
||||
} catch (error) {
|
||||
clearTimeout(timeout)
|
||||
writeFileSync(
|
||||
${JSON.stringify(paths.resultPath)},
|
||||
JSON.stringify({ error: String(error && error.stack ? error.stack : error) })
|
||||
)
|
||||
app.exit(1)
|
||||
}
|
||||
})
|
||||
`
|
||||
}
|
||||
|
||||
async function runFixture(): Promise<FixtureResult> {
|
||||
const root = mkdtempSync(join(tmpdir(), 'orca-ipc-boundary-bridge-'))
|
||||
fixtureRoots.push(root)
|
||||
const preloadSource = join(root, 'preload-entry.ts')
|
||||
const htmlPath = join(root, 'index.html')
|
||||
const mainPath = join(root, 'main.cjs')
|
||||
const resultPath = join(root, 'result.json')
|
||||
|
||||
writeFileSync(preloadSource, preloadEntry(join(process.cwd(), 'src/preload/ipc-invoke-boundary')))
|
||||
writeFileSync(htmlPath, `<!doctype html><body><script>${RENDERER_PROBE}</script></body>`)
|
||||
await buildVite({
|
||||
configFile: false,
|
||||
logLevel: 'silent',
|
||||
build: {
|
||||
emptyOutDir: false,
|
||||
// The fixture asserts on a constructor name, which minification would rewrite.
|
||||
minify: false,
|
||||
lib: {
|
||||
entry: preloadSource,
|
||||
formats: ['cjs'],
|
||||
fileName: () => 'preload.cjs',
|
||||
name: 'OrcaIpcInvokeBoundaryFixture'
|
||||
},
|
||||
outDir: root,
|
||||
target: 'node20',
|
||||
rollupOptions: { external: ['electron'] }
|
||||
}
|
||||
})
|
||||
writeFileSync(
|
||||
mainPath,
|
||||
fixtureMain({ htmlPath, preloadPath: join(root, 'preload.cjs'), resultPath })
|
||||
)
|
||||
|
||||
const { ELECTRON_RUN_AS_NODE: _electronRunAsNode, ...env } = process.env
|
||||
const electronArgs = [mainPath, `--user-data-dir=${join(root, 'profile')}`]
|
||||
const { executable, args } = resolveElectronProbeLaunch({
|
||||
electronBinary,
|
||||
electronArgs,
|
||||
platform: process.platform,
|
||||
display: env.DISPLAY
|
||||
})
|
||||
const run = spawnSync(executable, args, { encoding: 'utf8', env, timeout: 60_000 })
|
||||
const rawResult = existsSync(resultPath) ? readFileSync(resultPath, 'utf8') : 'no result'
|
||||
expect(run.error).toBeUndefined()
|
||||
expect(run.status, `${rawResult}\n${run.stdout}\n${run.stderr}`).toBe(0)
|
||||
return JSON.parse(rawResult) as FixtureResult
|
||||
}
|
||||
|
||||
describe('the stripped rejection as a renderer consumer receives it', () => {
|
||||
it('arrives as an Error carrying the reason, with the envelope only in the preload-side stack', async () => {
|
||||
const result = await runFixture()
|
||||
|
||||
// What the renderer can rely on.
|
||||
expect(result.rendererStripped.isError).toBe(true)
|
||||
expect(result.rendererStripped.message).toBe('Host key verification failed')
|
||||
|
||||
// The control proves the message and stack assertions above move when the strip does not run.
|
||||
expect(result.rendererUnstripped.message).toBe(
|
||||
"Error invoking remote method 'probe:reject-control': HandlerError: Host key verification failed"
|
||||
)
|
||||
expect(result.rendererUnstripped.stackFirstLine).toContain('Error invoking remote method')
|
||||
|
||||
// The renderer's stack is regenerated from the message, so it echoes the reason, not the
|
||||
// envelope. The wrapped form survives on the preload side, which is where it is logged.
|
||||
expect(result.rendererStripped.stackFirstLine).toBe('Error: Host key verification failed')
|
||||
expect(result.preloadStripped.stackFirstLine).toContain('Error invoking remote method')
|
||||
})
|
||||
|
||||
it('is a copy: prototype, own properties and object identity do not cross the bridge', async () => {
|
||||
const result = await runFixture()
|
||||
|
||||
// Preload holds a subclass with an own `code`; the renderer receives neither.
|
||||
expect(result.preloadCarriesOwnProperties.ctorName).toBe('BoundaryProbeError')
|
||||
expect(result.preloadCarriesOwnProperties.code).toBe('E_PROBE')
|
||||
expect(result.preloadCarriesOwnProperties.ownKeys).toContain('code')
|
||||
|
||||
expect(result.rendererCarriesOwnProperties.isError).toBe(true)
|
||||
expect(result.rendererCarriesOwnProperties.ctorName).toBe('Error')
|
||||
expect(result.rendererCarriesOwnProperties.name).toBe('Error')
|
||||
expect(result.rendererCarriesOwnProperties.code).toBeUndefined()
|
||||
expect(result.rendererCarriesOwnProperties.ownKeys).toEqual(['message', 'stack'])
|
||||
|
||||
// One preload object, rejected twice, arrives as two unrelated renderer objects.
|
||||
expect(result.sameObjectAcrossTwoRejections).toBe(false)
|
||||
expect(result.mutatingOneCopyLeaksToTheOther).toBe(false)
|
||||
|
||||
// An IPC rejection has nothing else to lose: it reaches the boundary already flattened.
|
||||
expect(result.preloadStripped.ctorName).toBe('Error')
|
||||
expect(result.preloadStripped.ownKeys).toEqual(['message', 'stack'])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,159 @@
|
||||
import { readdirSync, readFileSync, statSync } from 'node:fs'
|
||||
import { join, relative, resolve } from 'node:path'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
/**
|
||||
* Guard the envelope chokepoint at the tree level rather than per call site.
|
||||
*
|
||||
* `ipcRenderer.invoke` rejects with Electron's envelope, and the renderer's ordinary idiom renders
|
||||
* `err.message`. That made the leak unfixable per site: the shape that leaks is the shape that is
|
||||
* correct everywhere else, so a lint rule keyed on it fires on hundreds of sound lines. Routing the
|
||||
* 731 call sites through one wrapper fixed them at once — this test is what stops the 732nd from
|
||||
* being written outside it.
|
||||
*
|
||||
* ## What each assertion is worth
|
||||
*
|
||||
* Two of the three arms below are text scans, and a text scan cannot enumerate the ways JavaScript
|
||||
* spells a member access. Two separate bypasses have already been demonstrated against this file:
|
||||
* a cast with an aliased receiver, which a `window`-anchored pattern missed, and a computed access
|
||||
* whose keys are string literals (`w['electron']['ipcRenderer']['invoke']`), which the scan's own
|
||||
* string-blanking step erased before matching. The second one passed 3/3 green with a live escape
|
||||
* in the tree. Patching a third spelling would not change the shape: `'ipc' + 'Renderer'` walks
|
||||
* past any regex, and so does any key read from a variable.
|
||||
*
|
||||
* So the arms are labelled for what they are, not for what would be reassuring:
|
||||
*
|
||||
* - `stays behind the boundary` is a **fence**. `ipcRenderer` is only importable in preload, and
|
||||
* preload is three modules; a text scan is proportionate there and there is nowhere to hide.
|
||||
* - `the main world gets exactly these globals` is a **fence**, and the load-bearing one. Under
|
||||
* context isolation `exposeInMainWorld` is the only way to put anything in the renderer's world,
|
||||
* so the doors are enumerable, they all live in one file, and widening the set is a one-line diff
|
||||
* in the module a reviewer reads most closely.
|
||||
* - `is not reached through the raw bridge` is a **tripwire**, and is documented as one. It catches
|
||||
* somebody reaching for `window.electron.ipcRenderer` without thinking. It does not survive
|
||||
* somebody who means it, and it must not be read as though it does.
|
||||
*
|
||||
* The residue this cannot close: `window.electron` really is a live door to a raw `ipcRenderer`,
|
||||
* and no scan of renderer source will hold it shut. The only thing that closes it is not opening
|
||||
* it — nothing in the tree reads `window.electron` today, so the exposure could be dropped. That is
|
||||
* a change to the app's global surface rather than to this fix, so it is recorded here as the
|
||||
* standing recommendation and not smuggled in.
|
||||
*/
|
||||
const BOUNDARY_MODULE = 'src/preload/ipc-invoke-boundary.ts'
|
||||
const SRC_ROOT = resolve(__dirname, '..')
|
||||
const SCANNED_EXTENSIONS = ['.ts', '.tsx']
|
||||
const IGNORED_DIRECTORIES = new Set([
|
||||
'node_modules',
|
||||
'dist',
|
||||
'out',
|
||||
'build',
|
||||
'.git',
|
||||
'__fixtures__'
|
||||
])
|
||||
|
||||
/** Whitespace and newlines are legal between the receiver and the call, and one call site used them. */
|
||||
const RAW_INVOKE = /ipcRenderer\s*\.\s*invoke\s*\(/
|
||||
/**
|
||||
* Two spellings, both of which were live escapes before they were added, and neither of which makes
|
||||
* this arm complete — see the note above. Dot form is not anchored on `window`, because a cast sits
|
||||
* between `window` and `.electron`; the lookbehind keeps `electronFoo.ipcRenderer` out. Quoted-key
|
||||
* form is matched against source that still has its strings, because blanking them is exactly what
|
||||
* hid `['electron']['ipcRenderer']`. Typing the global closes neither: `Window.electron` IS declared
|
||||
* (`src/preload/api-types.ts`), so the plain spelling already compiles.
|
||||
*/
|
||||
const RAW_BRIDGE_DOT = /(?<!\w)electron\s*\.\s*ipcRenderer/
|
||||
const RAW_BRIDGE_COMPUTED = /\[\s*(['"`])ipcRenderer\1\s*\]/
|
||||
|
||||
/** The complete set of names preload puts in the renderer's world, by either code path. */
|
||||
const MAIN_WORLD_GLOBALS = ['api', 'electron']
|
||||
const PRELOAD_ENTRY = 'src/preload/index.ts'
|
||||
|
||||
/**
|
||||
* Comments name this shape on purpose — the modules that consume the envelope explain where it
|
||||
* comes from — so the scan reads code only. A ratchet that fired on prose would be silenced by
|
||||
* rewording rather than by fixing anything.
|
||||
*/
|
||||
function withoutComments(source: string): string {
|
||||
return source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/(^|[^:])\/\/[^\n]*/g, '$1')
|
||||
}
|
||||
|
||||
function withoutCommentsOrStrings(source: string): string {
|
||||
return withoutComments(source).replace(
|
||||
/'(?:[^'\\\n]|\\.)*'|"(?:[^"\\\n]|\\.)*"|`(?:[^`\\]|\\.)*`/g,
|
||||
"''"
|
||||
)
|
||||
}
|
||||
|
||||
/** Tests may reach for the raw call: they are not shipped, and several drive it to prove the wrapper. */
|
||||
function isTestFile(path: string): boolean {
|
||||
return /\.(?:test|spec)\.tsx?$/.test(path) || path.includes('/__tests__/')
|
||||
}
|
||||
|
||||
function collectSourceFiles(root: string): string[] {
|
||||
const found: string[] = []
|
||||
for (const entry of readdirSync(root)) {
|
||||
if (IGNORED_DIRECTORIES.has(entry)) {
|
||||
continue
|
||||
}
|
||||
const full = join(root, entry)
|
||||
if (statSync(full).isDirectory()) {
|
||||
found.push(...collectSourceFiles(full))
|
||||
} else if (SCANNED_EXTENSIONS.some((ext) => entry.endsWith(ext)) && !isTestFile(full)) {
|
||||
found.push(full)
|
||||
}
|
||||
}
|
||||
return found
|
||||
}
|
||||
|
||||
function offendingModules(pattern: RegExp, scrub = withoutCommentsOrStrings): string[] {
|
||||
return collectSourceFiles(SRC_ROOT)
|
||||
.filter((file) => pattern.test(scrub(readFileSync(file, 'utf8'))))
|
||||
.map((file) => relative(resolve(SRC_ROOT, '..'), file).replaceAll('\\', '/'))
|
||||
.sort()
|
||||
}
|
||||
|
||||
/** Every name preload hands the renderer: the `exposeInMainWorld` pair and the fallback assignment. */
|
||||
function exposedMainWorldGlobals(): string[] {
|
||||
const source = withoutComments(readFileSync(join(SRC_ROOT, 'preload', 'index.ts'), 'utf8'))
|
||||
const names = new Set<string>()
|
||||
for (const [, name] of source.matchAll(/exposeInMainWorld\s*\(\s*['"`]([\w$]+)['"`]/g)) {
|
||||
names.add(name)
|
||||
}
|
||||
for (const [, name] of source.matchAll(/(?:^|\n)\s*window\s*\.\s*([\w$]+)\s*=[^=]/g)) {
|
||||
names.add(name)
|
||||
}
|
||||
return [...names].sort()
|
||||
}
|
||||
|
||||
describe('ipcRenderer.invoke stays behind the preload boundary', () => {
|
||||
it('is called in exactly one module', () => {
|
||||
expect(offendingModules(RAW_INVOKE)).toEqual([BOUNDARY_MODULE])
|
||||
})
|
||||
|
||||
/**
|
||||
* A tripwire, not a fence. It reddens on the two spellings that were demonstrated against it and
|
||||
* on the obvious one; it does not claim to redden on a spelling nobody has written yet.
|
||||
*/
|
||||
it('is not reached through the raw electron bridge by any spelling this can see', () => {
|
||||
expect(offendingModules(RAW_BRIDGE_DOT)).toEqual([])
|
||||
expect(offendingModules(RAW_BRIDGE_COMPUTED, withoutComments)).toEqual([])
|
||||
})
|
||||
|
||||
/**
|
||||
* The arm that actually holds. `exposeInMainWorld` is the only way into an isolated renderer's
|
||||
* world, every call is in one file, and a new door has to be spelled out here to exist at all.
|
||||
*/
|
||||
it('gives the main world exactly these globals, from exactly one module', () => {
|
||||
expect(exposedMainWorldGlobals()).toEqual(MAIN_WORLD_GLOBALS)
|
||||
expect(offendingModules(/exposeInMainWorld\s*\(/)).toEqual([PRELOAD_ENTRY])
|
||||
})
|
||||
|
||||
/** A scan that matched nothing anywhere would pass both assertions above while enforcing nothing. */
|
||||
it('scans the modules it claims to', () => {
|
||||
const files = collectSourceFiles(SRC_ROOT)
|
||||
|
||||
expect(files.length).toBeGreaterThan(500)
|
||||
expect(files.some((file) => file.endsWith('preload/index.ts'))).toBe(true)
|
||||
expect(files.some((file) => file.endsWith('preload/gitlab.ts'))).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,142 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { invoke: rawInvoke } = vi.hoisted(() => ({ invoke: vi.fn() }))
|
||||
|
||||
vi.mock('electron', () => ({ ipcRenderer: { invoke: rawInvoke } }))
|
||||
|
||||
import { invoke, readableInvokeRejection } from './ipc-invoke-boundary'
|
||||
|
||||
/** The message the renderer would read after the boundary handled this rejection. */
|
||||
async function rejectionMessage(thrown: unknown): Promise<string> {
|
||||
rawInvoke.mockRejectedValueOnce(thrown)
|
||||
try {
|
||||
await invoke('workspaces:delete')
|
||||
throw new Error('expected the boundary to reject')
|
||||
} catch (error) {
|
||||
return (error as Error).message
|
||||
}
|
||||
}
|
||||
|
||||
describe('preload IPC invoke boundary', () => {
|
||||
let warn: ReturnType<typeof vi.spyOn>
|
||||
|
||||
beforeEach(() => {
|
||||
rawInvoke.mockReset()
|
||||
warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
})
|
||||
afterEach(() => {
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
it('resolves untouched, so wrapping costs the success path nothing', async () => {
|
||||
rawInvoke.mockResolvedValueOnce({ ok: true })
|
||||
|
||||
await expect(invoke('workspaces:delete', 'w1')).resolves.toEqual({ ok: true })
|
||||
expect(rawInvoke).toHaveBeenCalledWith('workspaces:delete', 'w1')
|
||||
})
|
||||
|
||||
describe('the shapes an envelope arrives in', () => {
|
||||
it('strips the renderer wrapper Electron rejects invoke with', async () => {
|
||||
const message = await rejectionMessage(
|
||||
new Error(
|
||||
"Error invoking remote method 'workspaces:delete': Error: Worktree has uncommitted changes"
|
||||
)
|
||||
)
|
||||
|
||||
expect(message).toBe('Worktree has uncommitted changes')
|
||||
})
|
||||
|
||||
it("strips main's handler wrapper", async () => {
|
||||
const message = await rejectionMessage(
|
||||
new Error(
|
||||
"Error occurred in handler for 'workspaces:delete': Error: Worktree has uncommitted changes"
|
||||
)
|
||||
)
|
||||
|
||||
expect(message).toBe('Worktree has uncommitted changes')
|
||||
})
|
||||
|
||||
/** A relay hop re-throws an already-wrapped message inside its own, so the envelope nests. */
|
||||
it('strips a relay re-throw that wrapped an already-wrapped message', async () => {
|
||||
const message = await rejectionMessage(
|
||||
new Error(
|
||||
"Error invoking remote method 'pty:connect': Error occurred in handler for 'pty:connect': Error: SSH connection lost, reconnecting"
|
||||
)
|
||||
)
|
||||
|
||||
expect(message).toBe('SSH connection lost, reconnecting')
|
||||
})
|
||||
})
|
||||
|
||||
describe('an envelope with nothing behind it', () => {
|
||||
/**
|
||||
* The tail is `error.toString()`, so a message-less rejection arrives as a bare class name.
|
||||
* Narrowing that to '' would render an empty toast — strictly worse than the plumbing — so the
|
||||
* boundary leaves it for the call site, which has copy naming what it was doing.
|
||||
*/
|
||||
it('leaves an empty tail alone rather than rejecting with an empty message', async () => {
|
||||
const wrapped = "Error invoking remote method 'workspaces:delete': "
|
||||
|
||||
expect(await rejectionMessage(new Error(wrapped))).toBe(wrapped)
|
||||
})
|
||||
|
||||
it('leaves an absent tail alone', async () => {
|
||||
const wrapped = "Error invoking remote method 'workspaces:delete'"
|
||||
|
||||
expect(await rejectionMessage(new Error(wrapped))).toBe(wrapped)
|
||||
})
|
||||
|
||||
it('leaves a bare class-name tail alone', async () => {
|
||||
const wrapped = "Error invoking remote method 'workspaces:delete': Error"
|
||||
|
||||
expect(await rejectionMessage(new Error(wrapped))).toBe(wrapped)
|
||||
})
|
||||
})
|
||||
|
||||
describe('what the boundary must not destroy', () => {
|
||||
it('keeps the wrapped form and the stack for diagnostics', async () => {
|
||||
const thrown = new Error(
|
||||
"Error invoking remote method 'workspaces:delete': Error: Worktree has uncommitted changes"
|
||||
)
|
||||
const stack = thrown.stack
|
||||
|
||||
rawInvoke.mockRejectedValueOnce(thrown)
|
||||
await expect(invoke('workspaces:delete')).rejects.toThrow('Worktree has uncommitted changes')
|
||||
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
"[ipc] 'workspaces:delete' rejected; raw:",
|
||||
"Error invoking remote method 'workspaces:delete': Error: Worktree has uncommitted changes",
|
||||
stack
|
||||
)
|
||||
// V8 fixes `stack` at construction, so the wrapped form survives on the error itself.
|
||||
expect(thrown.stack).toContain("Error invoking remote method 'workspaces:delete'")
|
||||
})
|
||||
|
||||
it('rejects with the same error object, so identity and properties survive', async () => {
|
||||
const thrown = Object.assign(
|
||||
new TypeError("Error invoking remote method 'git:push': Error: refusing to push"),
|
||||
{ code: 'EPUSH' }
|
||||
)
|
||||
|
||||
rawInvoke.mockRejectedValueOnce(thrown)
|
||||
const caught = await invoke('git:push').catch((error: unknown) => error)
|
||||
|
||||
expect(caught).toBe(thrown)
|
||||
expect(caught).toBeInstanceOf(TypeError)
|
||||
expect((caught as { code: string }).code).toBe('EPUSH')
|
||||
})
|
||||
|
||||
it('passes a non-Error rejection through untouched', () => {
|
||||
expect(readableInvokeRejection('plain string', 'git:push')).toBe('plain string')
|
||||
expect(readableInvokeRejection(undefined, 'git:push')).toBeUndefined()
|
||||
})
|
||||
|
||||
/** A message that never crossed IPC must not be logged as though it had. */
|
||||
it('leaves an unwrapped message alone and stays silent', async () => {
|
||||
expect(await rejectionMessage(new Error('Worktree has uncommitted changes'))).toBe(
|
||||
'Worktree has uncommitted changes'
|
||||
)
|
||||
expect(warn).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,61 @@
|
||||
/**
|
||||
* The one place a renderer-bound IPC rejection is read, so Electron's envelope is removed once.
|
||||
*
|
||||
* Electron names a rejected `ipcMain.handle` in the string it rejects with — "Error invoking remote
|
||||
* method '<channel>': <tail>" — and the renderer's ordinary idiom is to render `err.message`. That
|
||||
* idiom is correct everywhere else, so there is no per-call-site rule that separates the leaking
|
||||
* uses from the rest: the discriminator is whether the value crossed IPC, which is invisible at the
|
||||
* point it is rendered. Stripping here, where the envelope is created, is what makes the guarantee
|
||||
* hold for a call site nobody has written yet.
|
||||
*
|
||||
* The envelope is not lost, only demoted: it is logged here, against the channel that produced it,
|
||||
* before the message is narrowed. Preload is the last place it can be kept, because this rejection
|
||||
* does not reach the renderer as this object. `contextBridge` copies what crosses it, so a renderer
|
||||
* consumer receives a fresh plain `Error` carrying `message` and a `stack` regenerated from that
|
||||
* message — the prototype, own properties and object identity stop here, and so does the wrapped
|
||||
* form of the stack. Nothing is lost by narrowing in place that the bridge would not have dropped
|
||||
* anyway: an `ipcRenderer.invoke` rejection arrives already flattened to `message` and `stack`, its
|
||||
* own main-process class and properties gone one hop earlier.
|
||||
*
|
||||
* Measured across the real binary rather than reasoned about — see
|
||||
* `ipc-invoke-boundary-bridge.electron.test.ts`, which asserts the renderer's view.
|
||||
*/
|
||||
|
||||
import { ipcRenderer } from 'electron'
|
||||
import { stripIpcInvokeEnvelope } from '../shared/ipc-invoke-envelope'
|
||||
|
||||
/**
|
||||
* The rejection the renderer should see: the same error, carrying only the reason behind it.
|
||||
*
|
||||
* Left untouched when the envelope carried no readable reason — a handler that threw a message-less
|
||||
* error arrives as a bare class name, and an empty message renders as an empty toast, which is a
|
||||
* worse failure than the plumbing it replaces. Call sites that must never show plumbing already
|
||||
* branch on that case through `extractIpcErrorMessage`, which supplies copy this layer cannot know.
|
||||
*/
|
||||
export function readableInvokeRejection(rejection: unknown, channel: string): unknown {
|
||||
if (!(rejection instanceof Error)) {
|
||||
return rejection
|
||||
}
|
||||
const wrapped = rejection.message
|
||||
const reason = stripIpcInvokeEnvelope(wrapped)
|
||||
if (reason === null || reason === wrapped) {
|
||||
return rejection
|
||||
}
|
||||
console.warn(`[ipc] '${channel}' rejected; raw:`, wrapped, rejection.stack ?? '')
|
||||
rejection.message = reason
|
||||
return rejection
|
||||
}
|
||||
|
||||
/**
|
||||
* `ipcRenderer.invoke` with the envelope stripped from whatever it rejects with.
|
||||
*
|
||||
* `T` is inferred from the binding's declared return type, so this is type-neutral at the 731 call
|
||||
* sites that adopt it: the preload surface keeps saying what each channel resolves to.
|
||||
*/
|
||||
export async function invoke<T = unknown>(channel: string, ...args: unknown[]): Promise<T> {
|
||||
try {
|
||||
return (await ipcRenderer.invoke(channel, ...args)) as T
|
||||
} catch (rejection) {
|
||||
throw readableInvokeRejection(rejection, channel)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,136 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { PreloadApi } from './api-types'
|
||||
|
||||
const { exposeInMainWorld, invoke, on, removeListener, send, sendSync } = vi.hoisted(() => ({
|
||||
exposeInMainWorld: vi.fn(),
|
||||
invoke: vi.fn(),
|
||||
on: vi.fn(),
|
||||
removeListener: vi.fn(),
|
||||
send: vi.fn(),
|
||||
sendSync: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('electron', () => ({
|
||||
contextBridge: { exposeInMainWorld },
|
||||
ipcRenderer: { invoke, on, removeListener, send, sendSync },
|
||||
webFrame: {
|
||||
getZoomFactor: vi.fn(() => 1),
|
||||
setZoomFactor: vi.fn(),
|
||||
setVisualZoomLevelLimits: vi.fn()
|
||||
},
|
||||
webUtils: { getPathForFile: vi.fn(() => '') }
|
||||
}))
|
||||
|
||||
vi.mock('@electron-toolkit/preload', () => ({ electronAPI: {} }))
|
||||
|
||||
/**
|
||||
* The boundary claim measured on the real surface, not on the wrapper in isolation.
|
||||
*
|
||||
* Each case picks a binding a leaking call site actually reads — the toast copy quoted in the PR
|
||||
* came from `ssh.addTarget`, read at `SshPane.tsx:132` — rejects its channel with the envelope Electron produces, and asserts
|
||||
* the renderer never sees the plumbing. Driving `api` rather than the wrapper is the point: it is
|
||||
* what proves the 731 bindings are wired to it, not just that the wrapper works when called.
|
||||
*/
|
||||
describe('the preload surface strips the envelope for every binding', () => {
|
||||
const originalContextIsolated = Object.getOwnPropertyDescriptor(process, 'contextIsolated')
|
||||
let warn: ReturnType<typeof vi.spyOn>
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetModules()
|
||||
for (const spy of [exposeInMainWorld, invoke, on, removeListener, send, sendSync]) {
|
||||
spy.mockReset()
|
||||
}
|
||||
warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
Object.defineProperty(process, 'contextIsolated', { configurable: true, value: true })
|
||||
vi.stubGlobal('window', {
|
||||
addEventListener: vi.fn(),
|
||||
dispatchEvent: vi.fn(),
|
||||
removeEventListener: vi.fn()
|
||||
})
|
||||
vi.stubGlobal('document', { addEventListener: vi.fn() })
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
warn.mockRestore()
|
||||
vi.unstubAllGlobals()
|
||||
if (originalContextIsolated) {
|
||||
Object.defineProperty(process, 'contextIsolated', originalContextIsolated)
|
||||
} else {
|
||||
Reflect.deleteProperty(process, 'contextIsolated')
|
||||
}
|
||||
})
|
||||
|
||||
async function loadApi(): Promise<PreloadApi> {
|
||||
await import('./index')
|
||||
return exposeInMainWorld.mock.calls.find(([name]) => name === 'api')?.[1] as PreloadApi
|
||||
}
|
||||
|
||||
/** The message a renderer call site would put straight into a toast. */
|
||||
async function messageFrom(call: Promise<unknown>): Promise<string> {
|
||||
return await call.then(
|
||||
() => {
|
||||
throw new Error('expected the binding to reject')
|
||||
},
|
||||
(error: unknown) => (error as Error).message
|
||||
)
|
||||
}
|
||||
|
||||
it.each([
|
||||
[
|
||||
'ssh.addTarget',
|
||||
"Error invoking remote method 'ssh:addTarget': Error: Host key verification failed",
|
||||
'Host key verification failed'
|
||||
],
|
||||
[
|
||||
'worktrees.remove',
|
||||
"Error occurred in handler for 'worktrees:remove': Error: Worktree has uncommitted changes",
|
||||
'Worktree has uncommitted changes'
|
||||
],
|
||||
[
|
||||
'pty.connect (relay re-throw)',
|
||||
"Error invoking remote method 'pty:connect': Error occurred in handler for 'pty:connect': Error: SSH connection lost, reconnecting",
|
||||
'SSH connection lost, reconnecting'
|
||||
]
|
||||
])('%s reaches the renderer without the envelope', async (_label, wrapped, expected) => {
|
||||
invoke.mockRejectedValue(new Error(wrapped))
|
||||
const api = await loadApi()
|
||||
|
||||
await expect(
|
||||
messageFrom(api.ssh.addTarget({ target: {} as never }) as Promise<unknown>)
|
||||
).resolves.toBe(expected)
|
||||
})
|
||||
|
||||
it('leaves a reason-less rejection for the call site to name, rather than emptying it', async () => {
|
||||
const wrapped = "Error invoking remote method 'ssh:addTarget': Error"
|
||||
invoke.mockRejectedValue(new Error(wrapped))
|
||||
const api = await loadApi()
|
||||
|
||||
await expect(
|
||||
messageFrom(api.ssh.addTarget({ target: {} as never }) as Promise<unknown>)
|
||||
).resolves.toBe(wrapped)
|
||||
})
|
||||
|
||||
it('keeps the wrapped form on the log so diagnostics lose nothing', async () => {
|
||||
const wrapped =
|
||||
"Error invoking remote method 'ssh:addTarget': Error: Host key verification failed"
|
||||
invoke.mockRejectedValue(new Error(wrapped))
|
||||
const api = await loadApi()
|
||||
|
||||
await messageFrom(api.ssh.addTarget({ target: {} as never }) as Promise<unknown>)
|
||||
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
"[ipc] 'ssh:addTarget' rejected; raw:",
|
||||
wrapped,
|
||||
expect.stringContaining("Error invoking remote method 'ssh:addTarget'")
|
||||
)
|
||||
})
|
||||
|
||||
it('routes the GitLab bindings through the same boundary', async () => {
|
||||
invoke.mockRejectedValue(
|
||||
new Error("Error invoking remote method 'gitlab:viewer': Error: 401 Unauthorized")
|
||||
)
|
||||
const api = await loadApi()
|
||||
|
||||
await expect(messageFrom(api.gl.viewer() as Promise<unknown>)).resolves.toBe('401 Unauthorized')
|
||||
})
|
||||
})
|
||||
@@ -493,8 +493,9 @@ function TerminalPane(
|
||||
setSessionStateSaveFailureOpen(true)
|
||||
return
|
||||
}
|
||||
// Why: the surface renders the reason without Electron's IPC envelope, so the wrapped form —
|
||||
// which names the channel that failed — has to reach the log from here instead.
|
||||
// Why still here: the preload boundary strips rejections, but a pane error can also arrive over
|
||||
// an event channel, which never passes through it. This is the wrapped form's only log on that
|
||||
// path, so it narrowed rather than became dead.
|
||||
if (stripIpcInvokeEnvelope(message) !== message) {
|
||||
console.warn('[terminal] pane error reached the error surface IPC-wrapped; raw:', message)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,311 @@
|
||||
import { readdirSync, readFileSync, statSync } from 'node:fs'
|
||||
import { join, relative, resolve } from 'node:path'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
/**
|
||||
* The leaking population, computed rather than quoted.
|
||||
*
|
||||
* The case for the boundary rests on a number, and the number had only ever been written down: the
|
||||
* enumeration that produced it did not survive into the repository, so the figure in the PR body
|
||||
* could not be checked by anyone reading it. Three independent attempts on this population reported
|
||||
* 115, 118 and 137, which is what an unreproducible census looks like from outside. This file is the
|
||||
* census itself, so the figure is whatever running it says.
|
||||
*
|
||||
* ## The axis
|
||||
*
|
||||
* One axis, stated so a disagreeing count can be attributed instead of argued: **renderer
|
||||
* expressions that pass a rejection's free text into a render sink, in a module that talks to the
|
||||
* preload surface**. Concretely, an expression is counted when all of these hold:
|
||||
*
|
||||
* - it is an argument to a `toast.*(…)` call or to a `set<Name>(…)` state setter;
|
||||
* - it reads free text off a binding introduced by `catch (…)` or `.catch(…)` in the same module —
|
||||
* `binding.message`, `String(binding)` or `${binding}`;
|
||||
* - that argument does not route through `extractIpcErrorMessage` or `stripIpcInvokeEnvelope`;
|
||||
* - the module contains a `window.api.*` call, so a rejection reaching it can have crossed IPC.
|
||||
*
|
||||
* ## What it cannot see
|
||||
*
|
||||
* Stated rather than implied, because this population has been undercounted repeatedly. It is regex
|
||||
* over source, not dataflow, so it is a floor and not a total:
|
||||
*
|
||||
* - text laundered through an intermediate variable, a helper in another module, or a store action
|
||||
* not named `set*`, is invisible to it;
|
||||
* - its sinks are `toast.*` and `set*` only — direct JSX rendering of `{err.message}`, error
|
||||
* boundaries and `alert` are not counted;
|
||||
* - it cannot see event-channel payloads: `ipcRenderer.on` is not `invoke`, so a main-process string
|
||||
* arriving over an event is outside both this census and the fix;
|
||||
* - the `window.api.*` test is module-level, so a module that both calls the preload surface and
|
||||
* catches something else contributes a false positive, and a module that receives its rejection
|
||||
* from a caller contributes a false negative.
|
||||
*
|
||||
* ## Before and after
|
||||
*
|
||||
* The "before" is what this file computes. The "after" is not a second scan and cannot be: the fix
|
||||
* is upstream of every expression counted here, so the source is textually identical either side of
|
||||
* it and re-running the census would report the same number. What changes is the value that arrives.
|
||||
* The after-column is carried by two other running tests, and this file is only honest alongside
|
||||
* them: `ipc-invoke-boundary-bridge.electron.test.ts` shows a renderer consumer receiving the
|
||||
* narrowed reason across a real `contextBridge`, and `ipc-invoke-boundary-ratchet.test.ts` shows
|
||||
* that the wrapper is the only path to `ipcRenderer.invoke`, which is what makes that observation
|
||||
* general rather than anecdotal.
|
||||
*
|
||||
* To regenerate `LEAKING_EXPRESSIONS` after a legitimate change, run this file: the failure prints
|
||||
* the current population as a diff against the recorded one.
|
||||
*/
|
||||
const REPO_ROOT = resolve(__dirname, '../../../..')
|
||||
const RENDERER_ROOT = join(REPO_ROOT, 'src/renderer/src')
|
||||
const IGNORED_DIRECTORIES = new Set([
|
||||
'node_modules',
|
||||
'dist',
|
||||
'out',
|
||||
'build',
|
||||
'.git',
|
||||
'__fixtures__'
|
||||
])
|
||||
|
||||
/** Every leaking expression, by module. Computed by this file; not transcribed from anywhere. */
|
||||
const LEAKING_EXPRESSIONS: Readonly<Record<string, number>> = {
|
||||
'src/renderer/src/app-shell/use-app-session-persistence.ts': 1,
|
||||
'src/renderer/src/components/GitLabItemDialog.tsx': 2,
|
||||
'src/renderer/src/components/LinearItemDrawer.tsx': 1,
|
||||
'src/renderer/src/components/NewWorkspaceComposerCard.tsx': 1,
|
||||
'src/renderer/src/components/Terminal.tsx': 1,
|
||||
'src/renderer/src/components/browser-pane/ClientHostedBrowserPagePane.tsx': 1,
|
||||
'src/renderer/src/components/editor/useIpynbCellExecution.ts': 1,
|
||||
'src/renderer/src/components/emulator-pane/use-mobile-emulator-agent-setup-state.ts': 2,
|
||||
'src/renderer/src/components/feature-tips/CliSkillSetupTerminal.tsx': 1,
|
||||
'src/renderer/src/components/github-item-dialog/inspect-pull-request/checks-tab-actions.ts': 2,
|
||||
'src/renderer/src/components/github-item-dialog/land-pull-request/pr-actions-panel.tsx': 1,
|
||||
'src/renderer/src/components/github-project/slug-dialog/SlugDialogBody.tsx': 1,
|
||||
'src/renderer/src/components/jira-connect-dialog.tsx': 1,
|
||||
'src/renderer/src/components/linear-api-key-dialog.tsx': 1,
|
||||
'src/renderer/src/components/new-workspace/pick-local-project-folder.ts': 1,
|
||||
'src/renderer/src/components/onboarding/ThemeStep.tsx': 1,
|
||||
'src/renderer/src/components/onboarding/use-onboarding-flow-persistence.ts': 2,
|
||||
'src/renderer/src/components/pull-request-page/actions/merge-actions.ts': 3,
|
||||
'src/renderer/src/components/pull-request-page/checks/refresh.ts': 1,
|
||||
'src/renderer/src/components/pull-request-page/checks/rerun.ts': 1,
|
||||
'src/renderer/src/components/right-sidebar/ai-vault-session-launch-actions.ts': 1,
|
||||
'src/renderer/src/components/right-sidebar/ai-vault-session-refresh.ts': 1,
|
||||
'src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-create-review.tsx': 1,
|
||||
'src/renderer/src/components/right-sidebar/source-control/review/use-create-pr-intent-review.ts': 1,
|
||||
'src/renderer/src/components/right-sidebar/source-control/review/use-hosted-review-creation.ts': 1,
|
||||
'src/renderer/src/components/right-sidebar/source-control/sync/use-git-history-commit-actions.ts': 1,
|
||||
'src/renderer/src/components/right-sidebar/use-hosted-review-actions.ts': 2,
|
||||
'src/renderer/src/components/right-sidebar/useFileExplorerKeys.ts': 1,
|
||||
'src/renderer/src/components/settings/AgentSkillSetupPanel.tsx': 1,
|
||||
'src/renderer/src/components/settings/BrowserUseExamples.tsx': 1,
|
||||
'src/renderer/src/components/settings/BrowserUsePane.tsx': 1,
|
||||
'src/renderer/src/components/settings/CliSection.tsx': 3,
|
||||
'src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx': 1,
|
||||
'src/renderer/src/components/settings/ComputerUsePane.tsx': 3,
|
||||
'src/renderer/src/components/settings/EphemeralVmRuntimesSection.tsx': 4,
|
||||
'src/renderer/src/components/settings/EphemeralVmsPane.tsx': 1,
|
||||
'src/renderer/src/components/settings/GrokAccountsSection.tsx': 1,
|
||||
'src/renderer/src/components/settings/KeybindingsFileActions.tsx': 2,
|
||||
'src/renderer/src/components/settings/ManageSessionsSection.tsx': 2,
|
||||
'src/renderer/src/components/settings/MobileEmulatorAvailabilityDetails.tsx': 2,
|
||||
'src/renderer/src/components/settings/MobileEmulatorExamples.tsx': 1,
|
||||
'src/renderer/src/components/settings/OrchestrationSkillPromptDialog.tsx': 1,
|
||||
'src/renderer/src/components/settings/RepositoryIconTabs.tsx': 1,
|
||||
'src/renderer/src/components/settings/RuntimePairingUrlGenerator.tsx': 4,
|
||||
'src/renderer/src/components/settings/SkillUsageExampleDialog.tsx': 1,
|
||||
'src/renderer/src/components/settings/SshPane.tsx': 8,
|
||||
'src/renderer/src/components/settings/SshPassphraseDialog.tsx': 2,
|
||||
'src/renderer/src/components/settings/VoicePane.tsx': 2,
|
||||
'src/renderer/src/components/settings/WslCliRegistration.tsx': 3,
|
||||
'src/renderer/src/components/settings/bitbucket-credentials-dialog.tsx': 1,
|
||||
'src/renderer/src/components/settings/bitbucket-integration-card.tsx': 1,
|
||||
'src/renderer/src/components/settings/linear-agent-skill-install-cta.tsx': 1,
|
||||
'src/renderer/src/components/shared/useDaemonActions.tsx': 2,
|
||||
'src/renderer/src/components/sidebar/AddRemoteHostDialog.tsx': 2,
|
||||
'src/renderer/src/components/sidebar/AddRepoSteps.tsx': 1,
|
||||
'src/renderer/src/components/sidebar/ForgetSshWorkspaceDialog.tsx': 2,
|
||||
'src/renderer/src/components/sidebar/HostRemoveDialog.tsx': 1,
|
||||
'src/renderer/src/components/sidebar/HostSectionHeaderMenu.tsx': 2,
|
||||
'src/renderer/src/components/sidebar/NonGitFolderDialog.tsx': 1,
|
||||
'src/renderer/src/components/sidebar/SidebarSettingsHelpMenu.tsx': 1,
|
||||
'src/renderer/src/components/sidebar/WorktreeCardSshHostControl.tsx': 1,
|
||||
'src/renderer/src/components/sidebar/use-add-repo-host-selection.ts': 2,
|
||||
'src/renderer/src/components/sidebar/useSidebarProjectDrop.ts': 1,
|
||||
'src/renderer/src/components/status-bar/SshStatusSegment.tsx': 1,
|
||||
'src/renderer/src/components/status-bar/SshTargetStatusRow.tsx': 2,
|
||||
'src/renderer/src/components/tab-group/AiVaultSessionDropLayer.tsx': 1,
|
||||
'src/renderer/src/components/task-page/hooks/use-task-page-create-github-submit.ts': 1,
|
||||
'src/renderer/src/components/terminal-pane/TerminalSshReconnectOverlay.tsx': 1,
|
||||
'src/renderer/src/hooks/composer-state/attachment-drop-state.ts': 1,
|
||||
'src/renderer/src/hooks/composer-state/gitlab-provider-selection.ts': 1,
|
||||
'src/renderer/src/hooks/composer-state/host-runtime-effects.ts': 2,
|
||||
'src/renderer/src/hooks/ipc-events/content-creation-ipc-bridge.ts': 4,
|
||||
'src/renderer/src/hooks/ipc-events/direct-ssh-bridge-runtime.ts': 1,
|
||||
'src/renderer/src/hooks/ipc-events/remote-workspace-ipc-bridge.ts': 1,
|
||||
'src/renderer/src/hooks/useEphemeralVmRecipeOptions.ts': 1,
|
||||
'src/renderer/src/lib/agent-skill-cli-prerequisite.ts': 1,
|
||||
'src/renderer/src/lib/http-link-routing.ts': 1,
|
||||
'src/renderer/src/lib/launch-work-item-direct.ts': 1,
|
||||
'src/renderer/src/lib/sidebar-worktree-activation.ts': 1,
|
||||
'src/renderer/src/store/project-groups/nested-repository-operations.ts': 1,
|
||||
'src/renderer/src/store/repos/repo-removal.ts': 1,
|
||||
'src/renderer/src/store/slices/orca-profiles-auth-actions.ts': 5,
|
||||
'src/renderer/src/store/slices/orca-profiles.ts': 3,
|
||||
'src/renderer/src/store/slices/settings.ts': 1
|
||||
}
|
||||
|
||||
type Module = { path: string; source: string }
|
||||
|
||||
function isTestFile(path: string): boolean {
|
||||
return /\.(?:test|spec)\.tsx?$/.test(path) || path.includes('/__tests__/')
|
||||
}
|
||||
|
||||
function collectModules(root: string): Module[] {
|
||||
const found: Module[] = []
|
||||
for (const entry of readdirSync(root)) {
|
||||
if (IGNORED_DIRECTORIES.has(entry)) {
|
||||
continue
|
||||
}
|
||||
const full = join(root, entry)
|
||||
if (statSync(full).isDirectory()) {
|
||||
found.push(...collectModules(full))
|
||||
} else if ((entry.endsWith('.ts') || entry.endsWith('.tsx')) && !isTestFile(full)) {
|
||||
found.push({
|
||||
path: relative(REPO_ROOT, full).replaceAll('\\', '/'),
|
||||
source: readFileSync(full, 'utf8')
|
||||
})
|
||||
}
|
||||
}
|
||||
return found
|
||||
}
|
||||
|
||||
/** Strings go too: their contents are prose, and their parentheses would break argument balancing. */
|
||||
function withoutCommentsOrStringBodies(source: string): string {
|
||||
return source
|
||||
.replace(/\/\*[\s\S]*?\*\//g, '')
|
||||
.replace(/(^|[^:])\/\/[^\n]*/g, '$1')
|
||||
.replace(/'(?:[^'\\\n]|\\.)*'|"(?:[^"\\\n]|\\.)*"/g, '""')
|
||||
.replace(/`(?:[^`\\$]|\\.|\$(?!\{))*`/g, '""')
|
||||
}
|
||||
|
||||
/** Bindings a rejection can arrive on: `catch (e)` and the `.catch(e => …)` callback parameter. */
|
||||
function rejectionBindings(source: string): string[] {
|
||||
const names = new Set<string>()
|
||||
for (const [, name] of source.matchAll(/catch\s*\(\s*([A-Za-z_$][\w$]*)/g)) {
|
||||
names.add(name)
|
||||
}
|
||||
for (const [, name] of source.matchAll(/\.catch\s*\(\s*(?:async\s*)?\(?\s*([A-Za-z_$][\w$]*)/g)) {
|
||||
names.add(name)
|
||||
}
|
||||
names.delete('function')
|
||||
names.delete('async')
|
||||
return [...names]
|
||||
}
|
||||
|
||||
function balancedArgument(source: string, openParenIndex: number): string {
|
||||
let depth = 0
|
||||
for (let index = openParenIndex; index < source.length; index += 1) {
|
||||
if (source[index] === '(') {
|
||||
depth += 1
|
||||
} else if (source[index] === ')') {
|
||||
depth -= 1
|
||||
if (depth === 0) {
|
||||
return source.slice(openParenIndex + 1, index)
|
||||
}
|
||||
}
|
||||
}
|
||||
return source.slice(openParenIndex + 1)
|
||||
}
|
||||
|
||||
const SINK = /\btoast\s*(?:\.\s*[\w$]+)?\s*\(|\bset[A-Z][\w$]*\s*\(/g
|
||||
const ALREADY_STRIPPED = /extractIpcErrorMessage\s*\(|stripIpcInvokeEnvelope/
|
||||
const REACHES_PRELOAD = /\bwindow\s*\.\s*api\s*\./
|
||||
|
||||
export function censusLeakingExpressions(modules: readonly Module[]): Record<string, number> {
|
||||
const leaking: Record<string, number> = {}
|
||||
for (const { path, source } of modules) {
|
||||
const cleaned = withoutCommentsOrStringBodies(source)
|
||||
if (!REACHES_PRELOAD.test(cleaned)) {
|
||||
continue
|
||||
}
|
||||
const bindings = rejectionBindings(cleaned)
|
||||
if (bindings.length === 0) {
|
||||
continue
|
||||
}
|
||||
const alternation = bindings.join('|')
|
||||
const readsFreeText = new RegExp(
|
||||
`\\b(?:${alternation})\\b\\s*\\.\\s*message\\b` +
|
||||
`|String\\(\\s*(?:${alternation})\\s*\\)` +
|
||||
`|\\$\\{\\s*(?:${alternation})\\s*\\}`
|
||||
)
|
||||
SINK.lastIndex = 0
|
||||
while (SINK.exec(cleaned) !== null) {
|
||||
const argument = balancedArgument(cleaned, SINK.lastIndex - 1)
|
||||
if (readsFreeText.test(argument) && !ALREADY_STRIPPED.test(argument)) {
|
||||
leaking[path] = (leaking[path] ?? 0) + 1
|
||||
}
|
||||
}
|
||||
}
|
||||
return leaking
|
||||
}
|
||||
|
||||
/**
|
||||
* A module written to leak, injected rather than written to disk so the controls cannot leave the
|
||||
* tree mutated if the run is interrupted.
|
||||
*/
|
||||
const PLANTED_LEAK: Module = {
|
||||
path: 'src/renderer/src/planted-control.ts',
|
||||
source: `
|
||||
import { toast } from 'sonner'
|
||||
export async function planted(): Promise<void> {
|
||||
try {
|
||||
await window.api.ssh.addTarget()
|
||||
} catch (err) {
|
||||
toast.error(err instanceof Error ? err.message : String(err))
|
||||
}
|
||||
}
|
||||
`
|
||||
}
|
||||
|
||||
describe('the renderer expressions that would render an IPC envelope', () => {
|
||||
const treeModules = collectModules(RENDERER_ROOT)
|
||||
|
||||
it('are these, at these counts', () => {
|
||||
expect(censusLeakingExpressions(treeModules)).toEqual(LEAKING_EXPRESSIONS)
|
||||
})
|
||||
|
||||
it('total 131 expressions across 84 modules', () => {
|
||||
const census = censusLeakingExpressions(treeModules)
|
||||
const total = Object.values(census).reduce((sum, count) => sum + count, 0)
|
||||
|
||||
expect(total).toBe(131)
|
||||
expect(Object.keys(census)).toHaveLength(84)
|
||||
})
|
||||
|
||||
/**
|
||||
* The control the previous census never had: a census nobody has shown can detect the thing is
|
||||
* not evidence that the thing is absent.
|
||||
*/
|
||||
it('detects a planted leak, and counts exactly the one', () => {
|
||||
const before = censusLeakingExpressions(treeModules)
|
||||
const after = censusLeakingExpressions([...treeModules, PLANTED_LEAK])
|
||||
|
||||
expect(after[PLANTED_LEAK.path]).toBe(1)
|
||||
expect(Object.keys(after)).toHaveLength(Object.keys(before).length + 1)
|
||||
})
|
||||
|
||||
/** The other half of the control: the detector has to be able to say no, or it says nothing. */
|
||||
it('does not count the same expression once it is stripped, or outside the preload surface', () => {
|
||||
const stripped: Module = {
|
||||
path: PLANTED_LEAK.path,
|
||||
source: PLANTED_LEAK.source.replace(
|
||||
'err instanceof Error ? err.message : String(err)',
|
||||
'extractIpcErrorMessage(err)'
|
||||
)
|
||||
}
|
||||
const noPreloadCall: Module = {
|
||||
path: PLANTED_LEAK.path,
|
||||
source: PLANTED_LEAK.source.replace('window.api.ssh.addTarget()', 'somethingLocal()')
|
||||
}
|
||||
|
||||
expect(censusLeakingExpressions([stripped])).toEqual({})
|
||||
expect(censusLeakingExpressions([noPreloadCall])).toEqual({})
|
||||
})
|
||||
})
|
||||
@@ -124,6 +124,11 @@ const ENVELOPE_MODULE = 'src/shared/ipc-invoke-envelope.ts'
|
||||
* wrong for exactly this reason. Both lists together are the set of modules that open the envelope.
|
||||
*/
|
||||
const DIRECT_STRIPPER_SITES: Readonly<Record<string, { calls: number; surface: string }>> = {
|
||||
// The boundary itself: not a surface but the source of every message the rest of this list reads.
|
||||
'src/preload/ipc-invoke-boundary.ts': {
|
||||
calls: 1,
|
||||
surface: 'every preload binding — the rejection the renderer receives'
|
||||
},
|
||||
'src/renderer/src/components/LinuxPackageInstallRecoveryCard.tsx': {
|
||||
calls: 1,
|
||||
surface: 'recovery card'
|
||||
@@ -232,13 +237,17 @@ describe('extractIpcErrorMessage call sites', () => {
|
||||
* discriminator is whether the value crossed IPC, which is not visible where it is rendered. A
|
||||
* lint rule keyed on the idiom would fire on hundreds of correct sites and need suppressions.
|
||||
*
|
||||
* The narrow fix is the boundary, not the call site. Every envelope in the app is created in one
|
||||
* place — 730 `ipcRenderer.invoke(` calls live in exactly two files under `src/preload`. A preload
|
||||
* `invoke` wrapper that rejects with the stripped reason would fix all 118 at once and make this
|
||||
* census unnecessary, enforced by a ratchet test banning bare `ipcRenderer.invoke` outside it —
|
||||
* the same shape as the existing `child_process` ratchet. That is a separate change; the cost to
|
||||
* weigh first is that `TerminalPane.tsx` deliberately keeps the wrapped form in the console, so
|
||||
* the boundary must strip for display while the log keeps the original.
|
||||
* The narrow fix is the boundary, not the call site, and it has since been made: all 731
|
||||
* `ipcRenderer.invoke(` calls (702 + one written across two lines in `index.ts`, 28 in
|
||||
* `gitlab.ts`) now go through `src/preload/ipc-invoke-boundary.ts`, which rejects with the
|
||||
* stripped reason and logs the wrapped form against the channel that produced it. The count above
|
||||
* is what the boundary closed. `ipc-invoke-boundary-ratchet.test.ts` is what keeps the 732nd call
|
||||
* from being written outside it.
|
||||
*
|
||||
* This file stays as the change-detector it always was. It does not become the proof: it is keyed
|
||||
* on the stripper, so it still cannot see a site that does not strip — which is now the correct
|
||||
* state for a call site rather than a leak, because the value reaching it has already been
|
||||
* narrowed upstream.
|
||||
*/
|
||||
// Why: this is the number the freeze note got wrong, so it is asserted rather than described.
|
||||
it('number 31, and all of them render to a user', () => {
|
||||
|
||||
Reference in New Issue
Block a user