feat(sdk): custom function sync helpers - #321
Conversation
…n exchange, fetch/diff/sync)
@transcend-io/airgap.js-types
@transcend-io/cli
@transcend-io/design-tokens
@transcend-io/internationalization
@transcend-io/privacy-types
@transcend-io/sdk
@transcend-io/type-utils
@transcend-io/utils
@transcend-io/mcp
@transcend-io/mcp-server-admin
@transcend-io/mcp-server-assessment
@transcend-io/mcp-server-base
@transcend-io/mcp-server-consent
@transcend-io/mcp-server-discovery
@transcend-io/mcp-server-docs
@transcend-io/mcp-server-dsr
@transcend-io/mcp-server-inventory
@transcend-io/mcp-server-preferences
@transcend-io/mcp-server-workflows
commit: |
michaelfarrell76
left a comment
michaelfarrell76
left a comment
There was a problem hiding this comment.
see https://github.com/transcend-io/main/pull/46175#pullrequestreview-4732313911
will need to resolve this first and build a new sombra endpoint
…instead of employee session + DH
Automated Change: Auth pivot to customer-ingress signingPer security review feedback on the platform side (#46175), the signing flow no longer exchanges the API key for a Sombra employee session over a Diffie-Hellman channel. The CLI now signs code directly against the Sombra customer-ingress Implementation detailsWhat changed: All DH crypto and the session-exchange helper were removed from the SDK; a |
Automated Change: Per-gateway code signingCustom functions can belong to different Sombra gateways, and a function's JWTs must be signed by its own gateway's keys or they fail verification at execution time. Previously the push signed every function against a single gateway (the Implementation detailsWhat changed: The |
ecton-transcend
changed the title
|
|
||
| - if: github.event_name == 'pull_request' | ||
| name: Check changeset | ||
| env: |
There was a problem hiding this comment.
Hard coding the SHA causes long-open PRs to fail these check at the base_sha becomes stale. IMO safe to remove here as the fallback in code checks against main anyway.
If that was added for a specific reason I'm not aware of I'll add back in.
|
@michaelfarrell76 This is Branch 1 of a 3 part stacked PR sequence to introduce custom function management to the CLI:
Included in here the revised sombra-ingress route changes to avoid employee authentication sessions |
| @@ -0,0 +1,16 @@ | |||
| import { gql } from 'graphql-request'; | |||
|
|
|||
| export const ORGANIZATION_SOMBRAS = gql` | |||
There was a problem hiding this comment.
do we need this? isnt there an existing query for this?
There was a problem hiding this comment.
Good catch — removed
| return undefined; | ||
| } | ||
| try { | ||
| return JSON.parse(Buffer.from(segments[1], 'base64url').toString('utf-8')) as T; |
There was a problem hiding this comment.
why not use the jsonwebtoken lib?
| /** | ||
| * The product-facing type of a custom function. | ||
| */ | ||
| export type CustomFunctionType = 'DSR' | 'GENERAL'; |
There was a problem hiding this comment.
nit: these should probably be enums in privacy-types
| logger?: Logger; | ||
| } = {}, | ||
| ): Promise<string> { | ||
| const { logger = NOOP_LOGGER } = options; |
There was a problem hiding this comment.
pretty sure a function for this exists already!
There was a problem hiding this comment.
Oh yup, nice!
| * `enricher` export. GENERAL functions are always run as Maestro payloads and | ||
| * ignore this setting. | ||
| */ | ||
| export type CustomFunctionTestPayloadType = 'DATA_POINT' | 'REQUEST_ENRICHER'; |
There was a problem hiding this comment.
nit: enum in privacy-types
| }); | ||
| } | ||
|
|
||
| return { |
There was a problem hiding this comment.
nit: files a bit too big lets break it down
There was a problem hiding this comment.
Agreed, split the helpers up
|
@ecton-transcend terribly sorry for the delayed review! |
|
@ecton-transcend ya youre gtg, i could approve but as soon as you push another commit it will clear out the approval since this is a public package. so would just address the comments and ask for re-approval from @giacaglia this is close enough to merge but would do at least one round of cleanup befoer! |
…ck, don't retry codec validation errors
| let offset = 0; | ||
|
|
||
| let shouldContinue = false; | ||
| do { |
There was a problem hiding this comment.
nit: do while isn't quite stylistically in line, would prefer either plain while or a for loop
| draftVersion?: CustomFunctionVersionPreview | null; | ||
| } | ||
|
|
||
| const PAGE_SIZE = 20; |
There was a problem hiding this comment.
could/should this be larger? what's the max page size if we're fetching all? what's the volume per org? do we want to always fetch all attributes on the CFs?
| * functions with the same name) is an error that the caller should resolve by | ||
| * adding an `id` to the config. | ||
| * | ||
| * @param existing - All existing custom functions in the organization |
There was a problem hiding this comment.
does anything ensure that this is all of them? don't see any calling code, wondering the usefulness of this?
| } catch (err) { | ||
| const statusCode = (err as { response?: { statusCode?: number } })?.response?.statusCode; | ||
| if (statusCode === 404) { | ||
| throw new Error( |
There was a problem hiding this comment.
do we have a NotFoundError or anything like that in the SDK?
There was a problem hiding this comment.
or i guess whatever nnot supported is? 401?
| } = options; | ||
| const type: CustomFunctionType = input.type ?? CustomFunctionType.General; | ||
|
|
||
| const existing = resolveExistingCustomFunction(allExisting, input); |
There was a problem hiding this comment.
oh this is where it's called, it was folded up by github
There was a problem hiding this comment.
should be in a helpers folder probs
There was a problem hiding this comment.
if we're just using the allExisting to see if one exists. Can we just call an api with the id/name to get it back from the backend? This seems like a heavy way to go about it
There was a problem hiding this comment.
Good catch. It’s not just an existence check: we need the matched function’s signedCodeJwt / signedCodeContextJwt (to diff/skip), plus sombraId and dataSiloId.
The CLI fetches once per push and reuses that list across every manifest entry, so for a typical multi-function push one list call is cheaper than N lookups. Name matching also needs the full set so we can error when two functions share a name and the manifest has no id.
That said, customFunctions(filterBy: { id }) already exists on the backend, and for an id-keyed sync we could fetch just that one. Name-only still can’t be a clean “get by name” today: the API only has fuzzy text on name/description, not exact name. Happy to follow up with a targeted filterBy.id path if you want that cleaned up in this PR or a follow-up.
There was a problem hiding this comment.
yea I think if we're uniq-ing on name too then we should probably have a fetchByName api or a flag in the search filters for exact: true or something. I think it's significant enough that we shouldn't ship with a strategy of fetching all current functions to check for existence even though it's probably annoying to block on that?
if it's a multi-push as well you could check for conflicts within the set that you're pushing and then do a no-attribute fetch with filtered ids/names before pushing if you don't want to fail on submit. But yea overall pulling the full list in a loop is something we want to avoid for this
There was a problem hiding this comment.
signedCodeJwt / signedCodeContextJwt (to diff/skip), plus sombraId and dataSiloId.
should this happen in the backend?
There was a problem hiding this comment.
so for a typical multi-function push one list call is cheaper than N lookups
potentially but it does also loop over the pages to fetch them all which could add up in a CI situation
There was a problem hiding this comment.
The code is encrypted when fetched, so I think it's safe to pull down and run comparisons over what the user has locally. As the local code is encrypted at that point as well. Just checking if the hashes match.
I agree looping isn't the best mechanism here. I'll put together the preferred approach before merge!
There was a problem hiding this comment.
Yep no vuln concern :) just wanting to be a bit more optimal in how we hit typically pretty low-powered sombras lol. And maybe that is fetching, I'll approve and pass the ball back for your implementation 🙏
ty!!
| const existingCode = | ||
| codePayload === undefined | ||
| ? undefined | ||
| : Buffer.from(codePayload.base64Code, 'base64').toString('utf-8'); |
There was a problem hiding this comment.
naive question: is this just a node sdk or does it support browser too?
There was a problem hiding this comment.
also would this throw on empty string? or invalid base64?
| * @param token - The JWT string | ||
| * @returns The decoded payload object, or undefined if the JWT is malformed | ||
| */ | ||
| export function decodeJwtPayload<T extends object>(token: string): T | undefined { |
There was a problem hiding this comment.
nit: might not need to be own function, no real need to convert null to undefined here
| logger, | ||
| }); | ||
|
|
||
| const draft = updated.draftVersion; |
There was a problem hiding this comment.
nit: mixing various explicit undefined and truthy checks
| const draft = updated.draftVersion; | ||
| if (!draft) { | ||
| throw new Error( | ||
| `Expected a draft version to be created for custom function "${input.name}" but none was returned.`, |
There was a problem hiding this comment.
error could be more specific for an sdk, maybe throw earlier for more specific issues
| ...(input.allowThirdPartyImports !== undefined | ||
| ? { allowThirdPartyImports: input.allowThirdPartyImports } | ||
| : {}), | ||
| ...(input.timeoutMs !== undefined ? { timeoutMs: input.timeoutMs } : {}), |
There was a problem hiding this comment.
I think this could just be a default object with a spread on the input
There was a problem hiding this comment.
export function buildCustomFunctionSignPayload({
code,
context: contextOverrides,
}: CustomFunctionConfigInput): CustomFunctionSignPayload {
return {
code,
context: {
userDefinedEnv: {},
allowedHosts: [],
...contextOverrides,
},
};
}
|
The PR description notes "never in plaintext" for the code, but also states that just the env vars are encrypted. Are we expecting the code itself to also be encrypted? Being signed into a token is just it being signed which could be fine but I want to make sure I understand the description note on that one. |
The code bundles are passed into sombra via a custom-ingress route for encryption before being sent to the backend. So that comment is more saying the code/vars never reach transcend in plaintext. Which is the same security position we hold for AD implementation of the same feaures! |
eajoyce
left a comment
eajoyce
left a comment
There was a problem hiding this comment.
had a lingering thread about checking existing CFs, discussed options and leaving to ecton to take it forward. ty!
5 participants
Related Issues
Internal Changelog
custom-functionsmodule in@transcend-io/sdk— a customer-ingress code signing helper (signCustomFunctionCode, calling Sombra's new/v1/custom/signroute with bearer authentication), a Sombra gateway URL resolver (resolveSombraCustomerUrl), typed GraphQL documents, and fetch/diff/sync helpers for custom functions.Implementation details
How a sync works
syncCustomFunctionsends the plaintext code + execution context (allowed hosts, timeout, env vars) to the Sombra customer-ingress/v1/custom/signroute, authenticated by the Transcend API key bearer header (plus theX-Sombra-Authorizationinternal key when self-hosting) via the existingcreateSombraGotInstance. Sombra returns HMAC-signedsignedCodeJwt/signedCodeContextJwt.createCustomFunction/updateStandaloneCustomFunction(insideinput), where the API key'sManageDataMapscope is enforced. New revisions land as drafts and are promoted viapromoteCustomFunctionVersionunless promotion is disabled.updateStandaloneCustomFunctionhandles both GENERAL and DSR functions.Per-gateway signing
Each custom function belongs to a single Sombra gateway whose keys sign its code and encrypt its env values — signing against any other gateway would produce JWTs that fail verification at execution time.
resolveEffectiveSombraIdresolves the gateway per function (configsombraId→ existing function's gateway → caller default → primary), errors when a config tries to move an existing function between gateways, and thecustomFunctionsquery fetches each function'ssombraIdso updates always re-sign against the owning gateway.Matching and change detection
idfirst when set (rename-safe; a nonexistent ID fails rather than creating a duplicate), falling back to exact name. Ambiguous names (multiple functions sharing a name) fail with an error listing candidate IDs, since custom function names are not unique.customFunctionsquery and diffs code, allowed hosts, timeout, imports flag, and env variable names. Env values are encrypted at sign time and cannot be diffed — callers can force a re-sign for env-value rotations.Key files
packages/sdk/src/custom-functions/signCustomFunctionCode.tspackages/sdk/src/custom-functions/syncCustomFunction.ts,codeSigning.tspackages/sdk/src/custom-functions/resolveSombraCustomerUrl.tsDependencies / rollout
/v1/custom/signroute and thesignedCodeJwt/signedCodeContextJwtGraphQL input fields from the backend pivot (#46175). A 404 from an older Sombra produces a friendly "upgrade your gateway" error.Testing
Security Implications
ManageDataMapscope — no new privilege surface in this package.Test-before-promote (added in review)
runCustomFunctionTest— test-runs freshly signed (unsaved) code via therunCustomFunctionmutation using the pre-signedsignedCodeJwt/signedCodeContextJwtpair as a test run (isCustomFunctionTestRun: true), returning the execution result (error, logs, exit code, timing) plus apassedboolean that mirrors the dashboard's pass logic (no error and exit code ≤ 0).syncCustomFunctionaccepts optionaltestPayload/testPayloadType: when a payload is provided, the code is tested after signing and before any create/update. A failing test returns the newtest-failedoutcome with the fulltestResultattached, and nothing is pushed; passing runs continue and includetestResult.runCustomFunction(main #47383). On an older backend the mutation rejects the JWT input fields and the helper fails with a friendly upgrade error (callers can skip testing).Auto-created DSR integrations (added in review)
dataSiloId: when a DSR config has nodataSiloIdand no existing function matches,syncCustomFunctioncreates the DSR integration automatically — acustomFunction-catalog data silo shell (createCustomFunctionDataSilo, titled after the function, on the entry's gateway or the org's primary Sombra), created before the test run because the backend resolves the execution gateway from the payload'sextras.dataSilo.id.deleteDataSilo) and the sync returnstest-failedwithcreatedDataSilo: true.extras.dataSilo.{id,title}injected from the resolved silo (injectDataSiloIntoDsrTestPayload), so payload files never hardcode silo IDs.dataSiloId/createdDataSilo, and theCUSTOM_FUNCTIONSquery selectsdataSiloId, so callers can write silo IDs back to their manifests.Review follow-ups (Michael)
type/ lifecycle-state / payload-type string unions moved to@transcend-io/privacy-typesasCustomFunctionType,CustomFunctionLifecycleState,CustomFunctionVersionLifecycleState, andCustomFunctionPayloadType(wire values unchanged); the SDK and CLI now import them from there.decodeJwtPayloadis now a thin wrapper overjsonwebtoken'sjwt.decode(unsigned decode is intentional: change detection over JWTs fetched via the authenticated API).ORGANIZATION_SOMBRASquery andresolveSombraCustomerUrl; the existingORGANIZATIONquery now selectssombra { id customerUrl }+sombras { id customerUrl }, andcreateSombraGotInstancegained asombraIdoption to connect to a specific gateway. The CLI push flow passessombraIddirectly instead of pre-resolving a URL.syncCustomFunction.tswas split:resolveExistingCustomFunction.ts,resolveEffectiveSombraId.ts(incl.resolvePrimarySombraId),buildCustomFunctionSignPayload.ts, andinjectDataSiloIntoDsrTestPayload.tsare now their own modules; the orchestrator and result types stay.