Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/create-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,9 @@ jobs:

- name: Install dependencies
if: steps.check-published.outputs.already_published == 'false'
run: npm install
# The reviewed lockfile, with no dependency lifecycle scripts: this job
# holds the publishing identity, so nothing unreviewed runs in it.
run: npm ci --ignore-scripts

# Skip build and tests - this is a Typescript Client that users build
# The prepare script will handle necessary file generation for publishing
Expand Down
9 changes: 5 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,16 @@ jobs:
# Node 22 bundles npm 10.9, whose resolver can crash on a fresh dependency
# graph ("Cannot read properties of null (reading 'edgesOut')" in
# #loadPeerSet, 2026-09-03, first seen in robosystems-core). This lane
# verifies our code on the engines floor, not npm 10; with no lockfile,
# every run resolves from the live registry, so a current npm keeps the
# lane about Node rather than npm.
# verifies our code on the engines floor, not npm 10, so it runs the npm
# the release job uses.
- name: Use a current npm on the Node 22 lane
if: matrix.node-version == '22'
run: npm install -g npm@11

- name: Install Dependencies
run: npm install
# The committed lockfile, as the release job installs it: a lockfile out
# of step with package.json fails here rather than at publish.
run: npm ci

- name: Format Check
run: npm run format:check
Expand Down
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ Thumbs.db
tmp/

# Lock files
package-lock.json
yarn.lock
pnpm-lock.yaml

Expand Down
3 changes: 2 additions & 1 deletion clients/SSEClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
*/

import type { TokenProvider } from './graphql/client'
import { pathSegment } from './graphql/client'

export interface SSEConfig {
baseUrl: string
Expand Down Expand Up @@ -97,7 +98,7 @@ export class SSEClient {
if (reconnect && this.closed) return

return new Promise((resolve, reject) => {
let url = `${this.config.baseUrl}/v1/operations/${operationId}/stream?from_sequence=${fromSequence}`
let url = `${this.config.baseUrl}/v1/operations/${pathSegment(operationId, 'operationId')}/stream?from_sequence=${encodeURIComponent(String(fromSequence))}`

// Add JWT token as query parameter if provided
// WARNING: EventSource API doesn't support custom headers, so tokens are passed via query param
Expand Down
21 changes: 17 additions & 4 deletions clients/graphql/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,16 +205,29 @@ export async function resolveCredential(
}
}

const PATH_SEGMENT_PATTERN = /^[A-Za-z0-9_.-]{1,128}$/

/**
* Validate and encode a value interpolated into a request path as one segment.
* Anything that could change the route — a separator, dot segments, a query
* or fragment marker — is refused rather than escaped.
*/
export function pathSegment(value: string, name: string): string {
if (!value || !PATH_SEGMENT_PATTERN.test(value) || value === '.' || value === '..') {
throw new Error(`${name} must be a single path segment`)
}
return encodeURIComponent(value)
}

/**
* Build a new GraphQL client for the given graph. Prefer
* `GraphQLClientCache.get(graphId)` in facade code — this is the raw
* factory and bypasses the cache.
*/
export function createGraphQLClient(config: GraphQLClientConfig, graphId: string): GraphQLClient {
if (!graphId) {
throw new Error('createGraphQLClient requires a non-empty graphId')
}
const url = `${config.baseUrl.replace(/\/$/, '')}/extensions/${graphId}/graphql`
// One path segment, nothing else: a graph id from a URL parameter must not be
// able to re-target the request (and its credential) to another endpoint.
const url = `${config.baseUrl.replace(/\/$/, '')}/extensions/${pathSegment(graphId, 'graphId')}/graphql`
const staticHeaders: Record<string, string> = {
...(config.headers ?? {}),
}
Expand Down
41 changes: 41 additions & 0 deletions clients/pathSegment.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* A value interpolated into a request path is one segment or nothing: a graph
* id taken from a URL must not be able to point the request, and the
* credential attached to it, at another endpoint.
*/
import { describe, expect, it } from 'vitest'
import { createGraphQLClient, pathSegment } from './graphql/client'

describe('pathSegment', () => {
it('accepts graph ids as the API issues them', () => {
expect(pathSegment('kg0123456789abcdef', 'graphId')).toBe('kg0123456789abcdef')
expect(pathSegment('kg0123456789abcdef_dev', 'graphId')).toBe('kg0123456789abcdef_dev')
expect(pathSegment('sec', 'graphId')).toBe('sec')
})

it.each([
'../v1/graphs/kgDUMMYVICTIM/operations/materialize?',
'roboledger/kgDUMMYVICTIM/operations/accept-suggestions?',
'kg0123456789abcdef/graphql',
'kg0123456789abcdef?x=1',
'kg0123456789abcdef#frag',
'..',
'.',
'',
'kg 0123',
'a'.repeat(129),
])('refuses %j', (value) => {
expect(() => pathSegment(value, 'graphId')).toThrow('graphId must be a single path segment')
})
})

describe('createGraphQLClient', () => {
it('refuses a graph id that would re-target the request', () => {
expect(() =>
createGraphQLClient(
{ baseUrl: 'https://api.example.test' },
'../v1/graphs/kgDUMMYVICTIM/operations/materialize?'
)
).toThrow('graphId must be a single path segment')
})
})
Loading
Loading