Repository navigation
feat(connectors): export a single connector as JSON/YAML and import JSON/YAML files - #1202
helpcodeai merged 18 commits into
Conversation
Pure export helper reused by export-all; the import-all loop moves to a private method so a new import endpoint can share it. No behaviour change. Refs HelpCode-ai#1004
Single-connector export in the export-all envelope, as JSON or YAML (js-yaml core schema). Same secret masking for non-admins, authConfig never exported, 403 for another organization's connector. Refs HelpCode-ai#1004
Accepts { content } with the text of an export file, parses it with js-yaml (core schema), validates it against ImportAllDto with the global pipe options and imports it into the caller's organization through the shared importConnectors. Syntax errors return 400 with line and column; viewers get 403 before parsing. Refs HelpCode-ai#1004
Round trip into another organization for JSON and YAML with an OAuth connector and tool-level settings, masking for non-admins, authConfig never written, cross-org 403, viewer 403, line/column on YAML errors and unknown properties rejected as in import-all. Refs HelpCode-ai#1004
…files Connector page gets an Export menu (JSON or YAML) with a notice on secrets; the import dialog accepts .json/.yaml/.yml and sends the file text to POST /api/connectors/import. Refs HelpCode-ai#1004
A file holding a single connector object or an array of connectors is wrapped in the export envelope after yaml.load, for JSON and YAML alike, and then validated as before. Refs HelpCode-ai#1004
helpcodeai
left a comment
There was a problem hiding this comment.
Hi @GallaGit, thanks a lot, this is a really clean PR. The shared toExportEntry / importConnectors refactor is exactly what we wanted, the round-trip tests are good, and the e2e covers the right things. CI is green and I ran the two connector specs locally too.
Quick answers to your questions first:
- OAuth: keep it as it is. Exporting the non-secret OAuth keys is a separate piece of work, we'll open an issue for it.
- Tool overrides: fine as a known limit for this PR.
- Keep
POST /api/connectors/import, it matches the issue and there's no routing clash. - You read it right. Admin exports carry secret env vars and headers like export-all does, and the warning in the UI covers it. When I said secrets never end up in the file I meant authConfig and tokens.
One thing has to change before we can merge, plus a smaller one:
- YAML aliases. js-yaml resolves
*aliasto the same object, so a few hundred bytes of nested anchors parse instantly but blow up once class-transformer and Prisma walk the tree. I tried a 438 byte file with 6 levels of&l1 [*l0, *l0, ...]underconfig: it blocked the process for about 17 seconds and stored a 69 MB config, and one more level would run the server out of memory. On Cloud that's one process for every workspace, and any trial user can call this endpoint. A self-referencing anchor (config: &c {self: *c}) also ends in a stack overflow and a 500. Exports never write anchors (noRefs: true), so the simplest fix is to walk the parsed value once inparseImportFileand return 400 if any object or array shows up twice. That catches both cases. Please add a spec for each. - Hand-edited YAML can put numbers or booleans into
envVarsandheaders(PORT: 5432ends up stored as a number). Could you either turn those scalars into strings after parsing or reject them with a clear 400? Round trips are fine already, this is just for people who edit the file by hand.
Small optional stuff: the secretsIncluded regex in api.ts could be a response header from exportOne instead, and there's a stray blank line in connectors/page.tsx. No need to rebase, we'll update the branch before merging.
Thanks again, this is very close!
…vVars/headers on import parseImportFile now walks the parsed value once and returns 400 if it meets the same object or array twice. That covers alias bombs and self-referencing anchors. Number and boolean values in envVars and headers become strings, and object or array values are rejected with a 400.
The frontend reads the header instead of matching the file text with a regex. CORS now exposes Content-Disposition and X-Secrets-Included.
|
Thanks for the review! Changes are in the latest commits:
Backend typecheck and lint are clean, and all 36 export/import specs pass. Locally, |
helpcodeai
left a comment
There was a problem hiding this comment.
Thanks @GallaGit, the object alias fix and the scalar handling look good, and the specs are solid. I ran one more round on the YAML path and found a hole I should have mentioned the first time, sorry about that.
The alias check in connector-export.util.ts skips strings, so an anchored long string referenced many times still gets through. With a 1 MB string referenced 200k times inside config, the 1.6 MB file blocks the event loop for about 50 s and RSS goes to ~2.7 GB before Prisma gives up. A smaller one aliased inside a tool's endpointMapping (~100 KB file) is accepted and stores a ~100 MB value, which then gets loaded into the tool registry on every boot. Same thing through envVars and headers.
Could you give the walk a budget on the expanded size? Count string lengths and key lengths for every slot you visit (so each alias use counts again), and return a 400 once the total passes IMPORT_FILE_MAX_CHARS. Rejecting aliases altogether would also be fine with me, nobody needs them in a connector export. Please add a spec with a string alias bomb next to the existing ones.
Two smaller things while you're in there, both optional:
importConnectorscould callassertUsableBaseUrlon each connector'sbaseUrl. The request-time SSRF guard still protects us, but rejecting a metadata IP at import time is nicer.- In the connector page,
URL.revokeObjectURLright aftera.click()can cancel the download in some browsers. AsetTimeoutof a few seconds avoids it.
After that I'll update the branch and merge.
…on import The import walk now adds up the length of every string and object key, once per place it appears, and returns 400 once the total passes IMPORT_FILE_MAX_CHARS. This catches an anchored string repeated through aliases. importConnectors runs assertUsableBaseUrl on every connector before writing anything and returns 400 naming connectors[i].baseUrl.
|
Thanks again for the careful review! Addressed in two new commits:
Tests: backend typecheck and eslint are clean. The export/import spec passes, including the existing round-trip specs. In the connectors folder, 1557 tests pass. The one failure, |
helpcodeai
left a comment
There was a problem hiding this comment.
Checked the new walk: strings and keys now count once per place they appear, so the anchored-string case gets a 400 well before it hurts, and the baseUrl check runs before anything is written. Thanks for sticking with this through three rounds, merging.
Summary
This PR lets you export one connector and its tools as JSON or YAML, and import a JSON or YAML file back into the caller's organization. The export uses the same envelope as
export-all, so a single-connector file also restores throughimport-all. Both import endpoints share one code path and the existing DTO validation.Closes #1004. Part of the October Challenge #997.
Changes
GET /api/connectors/:id/export?format=json|yamlexports one connector in theexport-allenvelope (version,exportedAt,secretsIncluded,connectors: [entry]).CORE_SCHEMA(noRefs,lineWidth: -1) and sent asapplication/yaml; charset=utf-8.<slug>.anythingmcp.<json|yaml>.assertOrgMatch: another org's connector returns 403, and an unknown id returns 404.POST /api/connectors/importtakes{ "content": "<file text>" }.assertCanCreatebefore parsing.yaml.load(..., { schema: CORE_SCHEMA }), which also reads JSON. A syntax error returns 400 with the line and column.ImportAllDtowith the same options as the globalValidationPipe(transform,whitelist,forbidNonWhitelisted).toExportEntryandtoExportEnvelope(connector-export.util.ts, reusingtoPublicConnectorfromconnector-secrets.util.ts) are now shared byexport-alland:id/export.import-allloop moved to a privateimportConnectors()that both import endpoints use, so they apply the same licence/trial check per connector, skip duplicates by name within the caller's org, and return the same result shape.export-allandimport-allis unchanged.fetchand a Blob. After export, a notice says the file holds secrets in plain text (admins) or had them removed (other roles), and that OAuth credentials are not exported..json,.yaml,.yml, and now sends the raw text toPOST /api/connectors/importinstead of parsing JSON in the browser and callingimport-all.api.tshelpers:exportConnectorandimportFile.docs/api-reference.mdlistsexport-all,import-all,:id/exportandimport, with a YAML example.Validation that already existed still applies to the new endpoint. A file carrying
organizationIdorauthConfigon a connector, or an unknown tool property such asuseProxy, is rejected with 400 byforbidNonWhitelistedon the existing import DTOs, the same way as inimport-alland identically for JSON and YAML. That behaviour comes from the existing DTOs; this PR adds no new rule for it.Type
Testing
Added/updated unit tests
Tested manually (describe steps below)
No tests needed (explain why)
New
connectors.export-import.spec.ts(32 tests) together withconnectors.import-all.spec.ts(16 tests): 48/48 pass. They cover:authConfigabsent in both formats.cd packages/backend && npx jest src/connectors: 77 suites passed, 1 failed, 2 skipped. Tests: 1549 passed, 1 failed, 11 skipped (1561 total).engines/database.engine.ssrf.spec.ts › allows a blocked host when explicitly allowlisted(5000 ms timeout).mainat 99f04a1: 76 suites passed, 1 failed, 2 skipped. Tests: 1517 passed, 1 failed, 11 skipped (1529 total). Same test, same timeout.db.internalto fail quickly after it passes the SSRF guard. In my sandbox the DNS resolvesdb.internalto198.18.0.1(getent hosts db.internal), so the connection attempt hangs until Jest's 5 s timeout. In a normal environment the name does not resolve and the test fails fast as intended.Backend
tsc --noEmit: passes.eslinton the changed files: 0 errors, 0 warnings.Frontend
tsc --noEmit: passes.eslint src/: 0 errors, and the changed files have no new warnings (the existing warnings are unrelated).Playwright
tests/e2e/connector-export-import.spec.ts: 2/2 pass. They check that the Export menu requestsformat=yamland downloads the file, and that the import dialog accepts a.yamlfile and POSTs its text unchanged as{ content }.Checklist
npm test). All pass except the pre-existing environment-dependent SSRF test described above, which also fails on cleanmainhere.Known limits
authConfigis never exported, as the issue requires. That includes its non-secret parts (tokenUrl,authorizationUrl, grant, client id), so an imported OAuth connector keeps itsauthTypebut has to be configured and authorized again.useProxy, adminannotationsoverrides and tool role access. Only the 7 tool fields thatexport-allalready writes are exported (name,description,isEnabled,parameters,endpointMapping,responseMapping,outputSchema), andImportToolDtostill rejects the others. Per the issue, annotations need their own design.export-all), and the UI warns about it. Other roles get them masked.authConfigis never included.POST /api/connectors/importsits next to the existingPOST /api/connectors/:id/import(spec → tools). They don't clash, but the names are close. I can rename it toimport-fileif you prefer.contentis capped at 5,000,000 characters, within the existing 10 MB body limit.Related Issues
Closes #1004
Part of the October Challenge #997