fix(ipc): validate workload payload on utils:get-workload-available-tools handler#2037
Merged
fix(ipc): validate workload payload on utils:get-workload-available-tools handler#2037
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens the Electron IPC boundary for utils:get-workload-available-tools by validating the incoming workload payload against the generated CoreWorkload type (rather than a local ad-hoc interface), and by tightening the preload typing so renderer callers use a strongly typed API.
Changes:
- Add an
isCoreWorkloadruntime validator in the main-process IPC handler and reject malformed payloads with aTypeError. - Switch the preload
utils.getWorkloadAvailableToolsAPI from(workload: unknown)to(workload: CoreWorkload)to improve type-safety for callers. - Reuse
GithubComStacklokToolhivePkgCoreWorkloadfrom@common/api/generated/types.geninstead of redefining a parallel type.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
main/src/ipc-handlers/utils.ts |
Adds runtime validation for the IPC workload payload and rejects invalid inputs before calling getWorkloadAvailableTools. |
preload/src/api/utils.ts |
Tightens the preload API typing so getWorkloadAvailableTools requires a CoreWorkload at compile time. |
samuv
added a commit
that referenced
this pull request
Apr 17, 2026
…-tools Address Copilot review on #2037: the previous guard accepted any string for transport_type / proxy_mode and any number for port, so prototype keys like `__proto__` would fall through into createTransport, and `NaN` / non-http URLs could reach `new URL(...)` at the transport layer. - Restrict `transport_type` to {stdio, streamable-http, sse} and `proxy_mode` to {sse, streamable-http} via explicit allowlists - Require `port` to be a finite integer in [0, 65535] - Require `url` to be an http(s) URL parseable by `new URL` (empty string still tolerated; createTransport falls back to localhost)
…ools handler The handler previously forwarded the raw `workload` argument straight to getWorkloadAvailableTools, which expects the generated CoreWorkload type. An earlier attempt (#1978) introduced a local `Workload` interface with `name: string` and an `isWorkload` guard that only checked `typeof value' === `object`, so it neither matched the real type nor actually enforced a name. - Add `isCoreWorkload` that rejects non-objects, `null`, and arrays, and validates `name`, `url`, `transport_type`, `proxy_mode`, `port`, and `remote` match their declared types when present - Throw a `TypeError` at the IPC boundary for malformed payloads instead of forwarding them into the AI SDK / MCP client - Reuse `GithubComStacklokToolhivePkgCoreWorkload` from `@common/api/generated/types.gen` rather than defining a parallel type
…kload The preload wrapper and `UtilsAPI` interface both typed the workload parameter as `unknown`, which hid the real contract from callers and prevented the TypeScript compiler from catching misuse. The only caller (`customize-tools/page.tsx`) already passes a CoreWorkload, so no call-site changes are needed. - Import `GithubComStacklokToolhivePkgCoreWorkload` and use it for both the runtime wrapper and the exported `UtilsAPI` type
…-tools Address Copilot review on #2037: the previous guard accepted any string for transport_type / proxy_mode and any number for port, so prototype keys like `__proto__` would fall through into createTransport, and `NaN` / non-http URLs could reach `new URL(...)` at the transport layer. - Restrict `transport_type` to {stdio, streamable-http, sse} and `proxy_mode` to {sse, streamable-http} via explicit allowlists - Require `port` to be a finite integer in [0, 65535] - Require `url` to be an http(s) URL parseable by `new URL` (empty string still tolerated; createTransport falls back to localhost)
The main-process implementation returns `null` when `workload.name` is missing, but the preload API declared only `… | undefined`. Align the type so renderer callers see the real contract.
Add focused vitest coverage for the utils IPC handlers: rejection of non-object, prototype-key, out-of-range, and non-http payloads; happy paths for a full workload and an empty object; plus smoke tests for the other three handlers in the module.
samuv
force-pushed
the
fix/ipc-workload-validation
branch
from
de779e4 to
1737295
Compare
peppescg
approved these changes
Apr 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#1978 flagged that the
utils:get-workload-available-toolsIPC handler forwarded itsworkloadargument straight intogetWorkloadAvailableToolswithout validation. The fix in that PR introduced an ad-hocWorkloadinterface (name: string) that didn't match the realCoreWorkloadtype — wherenameis optional — and shipped anisWorkloadtype guard that only checkedtypeof value === 'object'while claiming to verify anameproperty (so thevalue is Workloadassertion was unsound). This PR replaces that with a real type guard against the generatedCoreWorkloadshape and tightens the preload API contract so callers aren't handedunknown.isCoreWorkloadinmain/src/ipc-handlers/utils.tsthat rejects non-objects,null, and arrays, and validatesname,url,transport_type,proxy_mode,port, andremotematch their declared types when present — the subset actually consumed bycreateTransport/getWorkloadAvailableToolsTypeErrorinstead of forwardingunknowninto the AI SDK / MCP client, while still allowing valid partial workloads (the consumer already handles!workload.nameby returningnull)GithubComStacklokToolhivePkgCoreWorkloadfrom@common/api/generated/types.genrather than redefining a parallel localWorkloadinterfacepreload/src/api/utils.tssoutils.getWorkloadAvailableTools(workload: CoreWorkload)is strongly typed on both the runtime wrapper and the exportedUtilsAPIinterface, instead ofunknown. The sole caller (renderer/.../customize-tools/page.tsx) already passes aCoreWorkload, so no downstream changes are requiredHow to validate
pnpm run type-checkandpnpm run lintpasswindow.electronAPI.utils.getWorkloadAvailableTools({})(or any non-object payload) should reject with aTypeErrorat the main-process boundary rather than reachingcreateTransportCoreWorkload— the only real caller path via the Customize Tools page — should behave exactly as beforeNot changed in this PR (deliberate)
getWorkloadAvailableToolsitself; its existing!workload.name → nullcontract is preservedipcMain.handle(...)call sites for similar unvalidated payloads can be a follow-upCloses #1978.