Skip to content

fix(worker): reject process global in workflow bundle#2128

Open
nkgotcode wants to merge 4 commits into
temporalio:mainfrom
nkgotcode:fix/reject-process-global-workflow-bundler
Open

fix(worker): reject process global in workflow bundle#2128
nkgotcode wants to merge 4 commits into
temporalio:mainfrom
nkgotcode:fix/reject-process-global-workflow-bundler

Conversation

@nkgotcode

@nkgotcode nkgotcode commented Jun 17, 2026

Copy link
Copy Markdown

Summary

Reject workflow bundles that reference the Node process global directly.

Webpack previously allowed free process references through the workflow bundle, which only failed later inside the workflow sandbox. This adds a workflow bundler parser check for free process expressions while preserving locally shadowed bindings.

Fixes #1112.

Testing

corepack pnpm install --frozen-lockfile
corepack pnpm -F @temporalio/worker run build
corepack pnpm -F @temporalio/test run build

Additional focused bundler checks were run with a minimal @temporalio/core-bridge cache stub:

  • a workflow referencing free process is rejected as a disallowed module
  • a workflow with a local process binding still bundles

Not run:

corepack pnpm -F @temporalio/worker... run build

This currently fails while building @temporalio/core-bridge because protoc is not installed in this environment.

@nkgotcode nkgotcode requested a review from a team as a code owner June 17, 2026 14:29
@CLAassistant

CLAassistant commented Jun 17, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@chris-olszewski chris-olszewski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this on. Only a small suggested change to avoid the as any cast.

Comment thread packages/worker/src/workflow/bundler.ts Outdated
Comment on lines +245 to +246
normalModuleFactory.hooks.parser.for(moduleType).tap('WorkflowCodeBundler', (parser) => {
const javascriptParser = parser as any;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will need to add import type { javascript } from 'webpack'

Suggested change
normalModuleFactory.hooks.parser.for(moduleType).tap('WorkflowCodeBundler', (parser) => {
const javascriptParser = parser as any;
normalModuleFactory.hooks.parser.for(moduleType).tap('WorkflowCodeBundler', (javascriptParser: javascript.JavascriptParser) => {

@nkgotcode

Copy link
Copy Markdown
Author

Addressed the review feedback in 448b528 by replacing the as any parser cast with javascript.JavascriptParser from webpack while preserving the hook callback signature required by the local webpack typings.\n\nValidation:\n- corepack pnpm install --frozen-lockfile\n- corepack pnpm -F @temporalio/worker run build still fails before completion because generated @temporalio/proto exports are missing in this fresh checkout; the previous bundler callback type error is gone after the update.\n- corepack pnpm exec tsc --noEmit --pretty false packages/worker/src/workflow/bundler.ts --skipLibCheck --esModuleInterop --moduleResolution node --target ES2022 --module commonjs still fails only on missing generated @temporalio/proto exports from imported worker utilities.

@chris-olszewski chris-olszewski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please read my suggested change from the previous review. You are still casting unnecessarily.

@nkgotcode nkgotcode force-pushed the fix/reject-process-global-workflow-bundler branch from 448b528 to 5bba954 Compare June 24, 2026 23:27
@nkgotcode

Copy link
Copy Markdown
Author

Updated the review fix in 5bba954 to follow the requested shape directly.\n\nWhat changed:\n- Removed the parser cast entirely.\n- The parser callback parameter is now typed directly as javascript.JavascriptParser.\n- Marked the JavaScript module type list as const so TypeScript selects webpack's JavaScript parser hook overload instead of the generic parser fallback.\n\nValidation:\n- corepack pnpm exec prettier --check packages/worker/src/workflow/bundler.ts passed.\n- corepack pnpm -F @temporalio/worker run build still fails in this local checkout because generated @temporalio/proto exports are unavailable, but the previous src/workflow/bundler.ts callback type error is gone after the as const overload fix.

@chris-olszewski chris-olszewski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need this new hook to respect our allowlist so that our internal testing where we colocate workflow code with tests can pass.

@nkgotcode

Copy link
Copy Markdown
Author

Addressed the allowlist feedback from @chris-olszewski.

Changes made:

  • The process parser hook now respects ignoreModules through the same module matching path used for disallowed imports.
  • Added process to baseBundlerIgnoreModules so colocated workflow/test code using the shared test helper allowlist can bundle.
  • Added a regression test that bundles a workflow referencing the process global with baseBundlerIgnoreModules.

Validation:

  • corepack pnpm exec prettier --check packages/worker/src/workflow/bundler.ts packages/test/src/test-bundler.ts packages/test-helpers/src/bundler.ts
  • corepack pnpm run build:protos
  • corepack pnpm -F @temporalio/test run build:protos
  • corepack pnpm -F @temporalio/worker -F @temporalio/test-helpers -F @temporalio/test run build:ts

I also tried the focused AVA bundler test locally, but this checkout is missing the packages/core-bridge/releases/aarch64-apple-darwin/index.node native prebuild, so AVA exits before test discovery.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Workflow Bundler does not deny usage of the process global variable

3 participants