fix(typert): validate and mount remote contributions safely
This commit is contained in:
@@ -4,7 +4,7 @@
|
|||||||
* lookup, invocation, or type exposure.
|
* lookup, invocation, or type exposure.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { Service } from 'cordis'
|
import { Service, symbols } from 'cordis'
|
||||||
import type { Context } from 'cordis'
|
import type { Context } from 'cordis'
|
||||||
import type { ConnectionHandle, RpcError } from '@deepseek-ai/dsh-client-connection/client'
|
import type { ConnectionHandle, RpcError } from '@deepseek-ai/dsh-client-connection/client'
|
||||||
import type {
|
import type {
|
||||||
@@ -84,7 +84,13 @@ class ClientApiService extends Service implements ClientApi {
|
|||||||
let disposeMethods: () => void | Promise<void>
|
let disposeMethods: () => void | Promise<void>
|
||||||
try {
|
try {
|
||||||
disposeMethods = callerCtx.effect(() => {
|
disposeMethods = callerCtx.effect(() => {
|
||||||
const installed = contribution.descriptors.map(descriptor => this.install(descriptor))
|
const installed: Array<() => void> = []
|
||||||
|
try {
|
||||||
|
for (const descriptor of contribution.descriptors) installed.push(this.install(descriptor))
|
||||||
|
} catch (error) {
|
||||||
|
for (const dispose of installed.reverse()) dispose()
|
||||||
|
throw error
|
||||||
|
}
|
||||||
return () => {
|
return () => {
|
||||||
for (const dispose of installed.reverse()) dispose()
|
for (const dispose of installed.reverse()) dispose()
|
||||||
}
|
}
|
||||||
@@ -169,21 +175,27 @@ class ClientApiService extends Service implements ClientApi {
|
|||||||
|
|
||||||
private installDirect(descriptor: InvocationDescriptor, token: MountToken): () => void {
|
private installDirect(descriptor: InvocationDescriptor, token: MountToken): () => void {
|
||||||
let namespace = this.direct.get(descriptor.namespace)
|
let namespace = this.direct.get(descriptor.namespace)
|
||||||
|
const fresh = namespace === undefined
|
||||||
if (namespace === undefined) {
|
if (namespace === undefined) {
|
||||||
namespace = { value: Object.create(null) as Record<string, RemoteMethod>, tokens: new Map() }
|
namespace = { value: Object.create(null) as Record<string, RemoteMethod>, tokens: new Map() }
|
||||||
this.direct.set(descriptor.namespace, namespace)
|
|
||||||
Object.defineProperty(this, descriptor.namespace, {
|
Object.defineProperty(this, descriptor.namespace, {
|
||||||
configurable: true,
|
configurable: true,
|
||||||
enumerable: true,
|
enumerable: true,
|
||||||
value: namespace.value,
|
value: namespace.value,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
try {
|
||||||
|
Object.defineProperty(namespace.value, descriptor.method, {
|
||||||
|
configurable: true,
|
||||||
|
enumerable: true,
|
||||||
|
value: (...args: unknown[]) => this.invoke(descriptor, undefined, token, this.ownerCtx, args),
|
||||||
|
})
|
||||||
|
} catch (error) {
|
||||||
|
if (fresh) Reflect.deleteProperty(this, descriptor.namespace)
|
||||||
|
throw error
|
||||||
|
}
|
||||||
|
if (fresh) this.direct.set(descriptor.namespace, namespace)
|
||||||
namespace.tokens.set(descriptor.method, token)
|
namespace.tokens.set(descriptor.method, token)
|
||||||
Object.defineProperty(namespace.value, descriptor.method, {
|
|
||||||
configurable: true,
|
|
||||||
enumerable: true,
|
|
||||||
value: (...args: unknown[]) => this.invoke(descriptor, undefined, token, this.ownerCtx, args),
|
|
||||||
})
|
|
||||||
return () => {
|
return () => {
|
||||||
/* v8 ignore next -- duplicate live methods are rejected before installation, so no newer token can replace this one. */
|
/* v8 ignore next -- duplicate live methods are rejected before installation, so no newer token can replace this one. */
|
||||||
if (namespace.tokens.get(descriptor.method) !== token) return
|
if (namespace.tokens.get(descriptor.method) !== token) return
|
||||||
@@ -242,7 +254,7 @@ class ClientApiService extends Service implements ClientApi {
|
|||||||
`client api: ${endpoint} expected ${contract}, got ${String(values.length)}`,
|
`client api: ${endpoint} expected ${contract}, got ${String(values.length)}`,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
const args: Record<string, unknown> = {}
|
const args = Object.create(null) as Record<string, unknown>
|
||||||
if (projection !== undefined) {
|
if (projection !== undefined) {
|
||||||
const binder = this.ownerCtx.typert.contexts.getClient(projection.context)
|
const binder = this.ownerCtx.typert.contexts.getClient(projection.context)
|
||||||
if (binder === undefined) {
|
if (binder === undefined) {
|
||||||
@@ -281,9 +293,12 @@ type InvokeRemote = (
|
|||||||
args: readonly unknown[],
|
args: readonly unknown[],
|
||||||
) => Promise<unknown>
|
) => Promise<unknown>
|
||||||
|
|
||||||
class ScopedRemoteNamespace extends Service {
|
class ScopedRemoteNamespace {
|
||||||
|
private readonly ctx: Context
|
||||||
private readonly ownerCtx: Context
|
private readonly ownerCtx: Context
|
||||||
private readonly methods = new Set<string>()
|
private readonly methods = new Set<string>()
|
||||||
|
private provided = false
|
||||||
|
readonly name: string
|
||||||
|
|
||||||
static assertMethodAvailable(namespace: string, method: string): void {
|
static assertMethodAvailable(namespace: string, method: string): void {
|
||||||
if (SCOPED_NAMESPACE_FIELDS.has(method) || method in ScopedRemoteNamespace.prototype) {
|
if (SCOPED_NAMESPACE_FIELDS.has(method) || method in ScopedRemoteNamespace.prototype) {
|
||||||
@@ -296,8 +311,12 @@ class ScopedRemoteNamespace extends Service {
|
|||||||
name: string,
|
name: string,
|
||||||
private readonly invokeRemote: InvokeRemote,
|
private readonly invokeRemote: InvokeRemote,
|
||||||
) {
|
) {
|
||||||
super(ctx, name)
|
this.ctx = ctx
|
||||||
this.ownerCtx = ctx
|
this.ownerCtx = ctx
|
||||||
|
this.name = name
|
||||||
|
Object.defineProperty(this, symbols.tracker, {
|
||||||
|
value: { associate: name, property: 'ctx' },
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
assertMethodAvailable(method: string): void {
|
assertMethodAvailable(method: string): void {
|
||||||
@@ -309,15 +328,28 @@ class ScopedRemoteNamespace extends Service {
|
|||||||
|
|
||||||
install(descriptor: InvocationDescriptor, projection: ScopedProjection, token: MountToken): void {
|
install(descriptor: InvocationDescriptor, projection: ScopedProjection, token: MountToken): void {
|
||||||
this.assertMethodAvailable(descriptor.method)
|
this.assertMethodAvailable(descriptor.method)
|
||||||
if (this.methods.size === 0) this.ownerCtx.set(this.name, this)
|
const activate = this.methods.size === 0
|
||||||
const method = descriptor.method
|
const method = descriptor.method
|
||||||
Object.defineProperty(this, method, {
|
try {
|
||||||
configurable: true,
|
Object.defineProperty(this, method, {
|
||||||
enumerable: true,
|
configurable: true,
|
||||||
value: function (this: ScopedRemoteNamespace, ...args: unknown[]): Promise<unknown> {
|
enumerable: true,
|
||||||
return this.invokeRemote(descriptor, projection, token, this.ctx, args)
|
value: function (this: ScopedRemoteNamespace, ...args: unknown[]): Promise<unknown> {
|
||||||
},
|
return this.invokeRemote(descriptor, projection, token, this.ctx, args)
|
||||||
})
|
},
|
||||||
|
})
|
||||||
|
if (activate) {
|
||||||
|
if (this.provided) {
|
||||||
|
this.ownerCtx.set(this.name, this)
|
||||||
|
} else {
|
||||||
|
this.ownerCtx.reflect.provide(this.name, this)
|
||||||
|
this.provided = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch (error) {
|
||||||
|
Reflect.deleteProperty(this, method)
|
||||||
|
throw error
|
||||||
|
}
|
||||||
this.methods.add(method)
|
this.methods.add(method)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -328,7 +360,7 @@ class ScopedRemoteNamespace extends Service {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const SCOPED_NAMESPACE_FIELDS = new Set(['ctx', 'invokeRemote', 'methods', 'name', 'ownerCtx'])
|
const SCOPED_NAMESPACE_FIELDS = new Set(['ctx', 'invokeRemote', 'methods', 'name', 'ownerCtx', 'provided'])
|
||||||
|
|
||||||
function endpointOf(descriptor: Pick<InvocationDescriptor, 'namespace' | 'method'>): string {
|
function endpointOf(descriptor: Pick<InvocationDescriptor, 'namespace' | 'method'>): string {
|
||||||
return `${descriptor.namespace}/${descriptor.method}`
|
return `${descriptor.namespace}/${descriptor.method}`
|
||||||
|
|||||||
@@ -321,6 +321,34 @@ describe('Client TypeRT API', () => {
|
|||||||
await disposeScoped()
|
await disposeScoped()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('rolls back earlier descriptors when a later descriptor fails to install', async () => {
|
||||||
|
const ctx = await bench(vi.fn<ConnectionHandle['rpc']['call']>())
|
||||||
|
const { scope: _scope, ...first } = directDescriptor()
|
||||||
|
const second: InvocationDescriptor = {
|
||||||
|
...first,
|
||||||
|
id: '@fixture/goals#goals/archive',
|
||||||
|
method: 'archive',
|
||||||
|
}
|
||||||
|
const defineProperty = Object.defineProperty
|
||||||
|
const spy = vi.spyOn(Object, 'defineProperty').mockImplementation((target, key, attributes) => {
|
||||||
|
if (key === 'archive') throw new Error('fixture later-descriptor failure')
|
||||||
|
return defineProperty(target, key, attributes)
|
||||||
|
})
|
||||||
|
try {
|
||||||
|
expect(() => ctx.api.mount({ package: '@fixture/failing-batch', descriptors: [first, second] }))
|
||||||
|
.toThrow('fixture later-descriptor failure')
|
||||||
|
} finally {
|
||||||
|
spy.mockRestore()
|
||||||
|
}
|
||||||
|
|
||||||
|
expect((ctx.api as unknown as Record<string, unknown>).goals).toBeUndefined()
|
||||||
|
await vi.waitFor(() => { expect(ctx.typert.remotes.list()).toEqual([]) })
|
||||||
|
const retry = ctx.api.mount({ package: '@fixture/retry-batch', descriptors: [first, second] })
|
||||||
|
expect(ctx.api.goals.create).toBeTypeOf('function')
|
||||||
|
expect((ctx.api.goals as unknown as Record<string, unknown>).archive).toBeTypeOf('function')
|
||||||
|
await retry()
|
||||||
|
})
|
||||||
|
|
||||||
it('rejects weak parameter and Context codecs plus malformed scope projections', async () => {
|
it('rejects weak parameter and Context codecs plus malformed scope projections', async () => {
|
||||||
const ctx = await bench(vi.fn<ConnectionHandle['rpc']['call']>())
|
const ctx = await bench(vi.fn<ConnectionHandle['rpc']['call']>())
|
||||||
const direct = directDescriptor()
|
const direct = directDescriptor()
|
||||||
@@ -409,6 +437,33 @@ describe('Client TypeRT API', () => {
|
|||||||
expect((ctx.api as unknown as Record<string, unknown>).goals).toBeUndefined()
|
expect((ctx.api as unknown as Record<string, unknown>).goals).toBeUndefined()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('preserves a __proto__ wire parameter as an own named argument', async () => {
|
||||||
|
const call = vi.fn<ConnectionHandle['rpc']['call']>()
|
||||||
|
.mockResolvedValue({ ok: true, value: { ref: 'goal-1' } })
|
||||||
|
const ctx = await bench(call)
|
||||||
|
const { scope: _scope, ...base } = directDescriptor()
|
||||||
|
const descriptor: InvocationDescriptor = {
|
||||||
|
...base,
|
||||||
|
id: '@fixture/goals#goals/prototype',
|
||||||
|
method: 'prototype',
|
||||||
|
parameters: [{
|
||||||
|
name: 'value',
|
||||||
|
wire: '__proto__',
|
||||||
|
source: 'json',
|
||||||
|
codec: { mode: 'strict', typeSymbol: '@fixture#PrototypeValue', schema: z.string() },
|
||||||
|
}],
|
||||||
|
}
|
||||||
|
const dispose = ctx.api.mount({ package: '@fixture/prototype', descriptors: [descriptor] })
|
||||||
|
|
||||||
|
const method = (ctx.api.goals as unknown as Record<string, (...args: unknown[]) => Promise<unknown>>).prototype
|
||||||
|
await expect(method?.('wire-value')).resolves.toEqual({ ref: 'goal-1' })
|
||||||
|
const payload = call.mock.calls[0]?.[2] as { readonly args: Record<string, unknown> }
|
||||||
|
expect(Object.getPrototypeOf(payload.args)).toBeNull()
|
||||||
|
expect(Object.hasOwn(payload.args, '__proto__')).toBe(true)
|
||||||
|
expect(payload.args.__proto__).toBe('wire-value')
|
||||||
|
await dispose()
|
||||||
|
})
|
||||||
|
|
||||||
it('rolls back Remote registration when concrete method installation fails', async () => {
|
it('rolls back Remote registration when concrete method installation fails', async () => {
|
||||||
const ctx = await bench(vi.fn<ConnectionHandle['rpc']['call']>())
|
const ctx = await bench(vi.fn<ConnectionHandle['rpc']['call']>())
|
||||||
const defineProperty = Object.defineProperty
|
const defineProperty = Object.defineProperty
|
||||||
@@ -423,6 +478,31 @@ describe('Client TypeRT API', () => {
|
|||||||
} finally {
|
} finally {
|
||||||
spy.mockRestore()
|
spy.mockRestore()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const retry = ctx.api.mount({ package: '@fixture/goals-retry', descriptors: [directDescriptor()] })
|
||||||
|
expect(ctx.api.goals.create).toBeTypeOf('function')
|
||||||
|
await retry()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('withdraws a fresh scoped Service when its first method fails to install', async () => {
|
||||||
|
const ctx = await bench(vi.fn<ConnectionHandle['rpc']['call']>())
|
||||||
|
const defineProperty = Object.defineProperty
|
||||||
|
const spy = vi.spyOn(Object, 'defineProperty').mockImplementation((target, key, attributes) => {
|
||||||
|
if (key === 'rename') throw new Error('fixture scoped installation failure')
|
||||||
|
return defineProperty(target, key, attributes)
|
||||||
|
})
|
||||||
|
try {
|
||||||
|
expect(() => ctx.api.mount({ package: '@fixture/scoped-failure', descriptors: [contextDescriptor()] }))
|
||||||
|
.toThrow('fixture scoped installation failure')
|
||||||
|
} finally {
|
||||||
|
spy.mockRestore()
|
||||||
|
}
|
||||||
|
|
||||||
|
expect(ctx.get('goals')).toBeUndefined()
|
||||||
|
await vi.waitFor(() => { expect(ctx.typert.remotes.list()).toEqual([]) })
|
||||||
|
const retry = ctx.api.mount({ package: '@fixture/scoped-retry', descriptors: [contextDescriptor()] })
|
||||||
|
expect((ctx.get('goals') as unknown as Record<string, unknown>).rename).toBeTypeOf('function')
|
||||||
|
await retry()
|
||||||
})
|
})
|
||||||
|
|
||||||
it('throws RPC failures with the structured error as its cause', async () => {
|
it('throws RPC failures with the structured error as its cause', async () => {
|
||||||
|
|||||||
@@ -2810,7 +2810,9 @@ function stringLiteralValue(node: ts.Node | undefined): string | undefined {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function isRemoteSegment(value: string): boolean {
|
function isRemoteSegment(value: string): boolean {
|
||||||
return /^[A-Za-z0-9_$.-]+$/.test(value)
|
// Generation bootstraps workspace artifacts before dsh-type-meta is built,
|
||||||
|
// so this extraction-only copy must mirror isTypeRTRemoteSegment().
|
||||||
|
return value !== '.' && value !== '..' && /^[A-Za-z0-9_$.-]+$/.test(value)
|
||||||
}
|
}
|
||||||
|
|
||||||
function expressionName(node: ts.Expression): string | undefined {
|
function expressionName(node: ts.Expression): string | undefined {
|
||||||
|
|||||||
@@ -90,6 +90,7 @@ export class WorkspaceTypertGenerator {
|
|||||||
throw new TypertAnalysisError(`typert(${artifact.face}): ${artifact.package} package files must include ${file}`)
|
throw new TypertAnalysisError(`typert(${artifact.face}): ${artifact.package} package files must include ${file}`)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if (artifact.face !== 'host') return
|
||||||
const remoteExpected = {
|
const remoteExpected = {
|
||||||
types: './lib/typert.remote-client.d.ts',
|
types: './lib/typert.remote-client.d.ts',
|
||||||
default: './lib/typert.remote-client.js',
|
default: './lib/typert.remote-client.js',
|
||||||
|
|||||||
@@ -274,7 +274,7 @@ export interface BoxPayload {
|
|||||||
assertRemoteConsumerTypechecks(artifact?.remote?.dts, artifact?.remote?.dtsMap, root)
|
assertRemoteConsumerTypechecks(artifact?.remote?.dts, artifact?.remote?.dtsMap, root)
|
||||||
})
|
})
|
||||||
|
|
||||||
it.each(['create#v2', 'create goal'])('rejects untransportable Remote alias %s', (alias) => {
|
it.each(['create#v2', 'create goal', '.', '..'])('rejects untransportable Remote alias %s', (alias) => {
|
||||||
const root = copyFixture()
|
const root = copyFixture()
|
||||||
editFile(root, 'packages/remote/src/index.ts', source => source.replace(
|
editFile(root, 'packages/remote/src/index.ts', source => source.replace(
|
||||||
' @Remote\n async create(',
|
' @Remote\n async create(',
|
||||||
@@ -301,6 +301,37 @@ export interface RemainingSchema {
|
|||||||
.toThrow('publishes Remote artifacts but has no Remote methods')
|
.toThrow('publishes Remote artifacts but has no Remote methods')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('validates Remote artifacts only on the host face of a dual-face package', () => {
|
||||||
|
const root = copyFixture()
|
||||||
|
const manifestPath = join(root, 'packages/remote/package.json')
|
||||||
|
const manifest = JSON.parse(readFileSync(manifestPath, 'utf8')) as {
|
||||||
|
dshClient?: object
|
||||||
|
exports: Record<string, unknown>
|
||||||
|
files: string[]
|
||||||
|
}
|
||||||
|
manifest.dshClient = {}
|
||||||
|
manifest.exports['./client'] = './src/client.ts'
|
||||||
|
manifest.exports['./client/typert'] = {
|
||||||
|
types: './lib/typert.client.d.ts',
|
||||||
|
default: './lib/typert.client.js',
|
||||||
|
}
|
||||||
|
manifest.files.push('lib/typert.client.js', 'lib/typert.client.d.ts')
|
||||||
|
writeFileSync(manifestPath, `${JSON.stringify(manifest, null, 2)}\n`)
|
||||||
|
writeFileSync(join(root, 'tsconfig.client.json'), `${JSON.stringify({
|
||||||
|
extends: './tsconfig.base.json',
|
||||||
|
files: [],
|
||||||
|
references: [{ path: './packages/remote' }],
|
||||||
|
}, null, 2)}\n`)
|
||||||
|
writeFileSync(join(root, 'packages/remote/src/client.ts'), `/** @typert schema */
|
||||||
|
export interface ClientMarker {
|
||||||
|
readonly ready: boolean
|
||||||
|
}
|
||||||
|
`)
|
||||||
|
|
||||||
|
expect(new WorkspaceTypertGenerator(root).generate().map(artifact => artifact.face))
|
||||||
|
.toEqual(['host', 'client'])
|
||||||
|
})
|
||||||
|
|
||||||
it.each([
|
it.each([
|
||||||
{
|
{
|
||||||
name: 'missing binding',
|
name: 'missing binding',
|
||||||
|
|||||||
@@ -600,7 +600,7 @@ function validateCodec(codec: InvocationDescriptor['result'], subject: string):
|
|||||||
}
|
}
|
||||||
|
|
||||||
function validateWireName(subject: string, value: string): void {
|
function validateWireName(subject: string, value: string): void {
|
||||||
if (!/^[A-Za-z0-9_$.-]+$/.test(value)) {
|
if (value === '.' || value === '..' || !/^[A-Za-z0-9_$.-]+$/.test(value)) {
|
||||||
throw new Error(`typert: invalid ${subject} "${value}" — must contain only RPC endpoint segment characters`)
|
throw new Error(`typert: invalid ${subject} "${value}" — must contain only RPC endpoint segment characters`)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -247,7 +247,7 @@ describe('TypertRegistry', () => {
|
|||||||
})).toThrow('endpoint "goals/create" is already registered')
|
})).toThrow('endpoint "goals/create" is already registered')
|
||||||
})
|
})
|
||||||
|
|
||||||
it.each(['create#v2', 'create goal'])('rejects untransportable invocation method %s', async (method) => {
|
it.each(['create#v2', 'create goal', '.', '..'])('rejects untransportable invocation method %s', async (method) => {
|
||||||
const ctx = await makeCtx()
|
const ctx = await makeCtx()
|
||||||
expect(() => ctx.typert.remotes.register({
|
expect(() => ctx.typert.remotes.register({
|
||||||
package: '@fixture/invalid-endpoint',
|
package: '@fixture/invalid-endpoint',
|
||||||
|
|||||||
@@ -15,7 +15,7 @@ const TYPERT_REMOTE_SEGMENT_PATTERN = /^[A-Za-z0-9_$.-]+$/
|
|||||||
* @returns whether the value can cross the shared RPC carrier unchanged.
|
* @returns whether the value can cross the shared RPC carrier unchanged.
|
||||||
*/
|
*/
|
||||||
export function isTypeRTRemoteSegment(value: string): boolean {
|
export function isTypeRTRemoteSegment(value: string): boolean {
|
||||||
return TYPERT_REMOTE_SEGMENT_PATTERN.test(value)
|
return value !== '.' && value !== '..' && TYPERT_REMOTE_SEGMENT_PATTERN.test(value)
|
||||||
}
|
}
|
||||||
|
|
||||||
export type {
|
export type {
|
||||||
|
|||||||
@@ -164,6 +164,8 @@ describe('type-meta Remote declarations', () => {
|
|||||||
expect(() => Remote('bad/name')).toThrow('export name')
|
expect(() => Remote('bad/name')).toThrow('export name')
|
||||||
expect(() => Remote('bad#name')).toThrow('export name')
|
expect(() => Remote('bad#name')).toThrow('export name')
|
||||||
expect(() => Remote('bad name')).toThrow('export name')
|
expect(() => Remote('bad name')).toThrow('export name')
|
||||||
|
expect(() => Remote('.')).toThrow('export name')
|
||||||
|
expect(() => Remote('..')).toThrow('export name')
|
||||||
expect(() => RemoteContext('' as 'metaFixture')).toThrow('Context key')
|
expect(() => RemoteContext('' as 'metaFixture')).toThrow('Context key')
|
||||||
expect(() => RemoteContext('metaFixture', 'bad/name')).toThrow('export name')
|
expect(() => RemoteContext('metaFixture', 'bad/name')).toThrow('export name')
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user