Skip to content

feat(cli): warn on createRequire packages missing from deployed images - #3

Open
anurag6569201 wants to merge 1 commit into
qa/agent-triggerdotdev-trigger-dev/pr-03-4851/basefrom
qa/agent-triggerdotdev-trigger-dev/pr-03-4851/head
Open

feat(cli): warn on createRequire packages missing from deployed images#3
anurag6569201 wants to merge 1 commit into
qa/agent-triggerdotdev-trigger-dev/pr-03-4851/basefrom
qa/agent-triggerdotdev-trigger-dev/pr-03-4851/head

Conversation

@anurag6569201

Copy link
Copy Markdown

Summary

The trigger.dev deploy and trigger.dev dev commands now warn (with the suggested fix) when your code loads a package through createRequire() that won't be available in the deployed image. Previously it would fail at runtime in production to load the package. Deploys also now show bundler warnings for your code instead of discarding them.

A package loaded with createRequire(import.meta.url)("pkg") is invisible to esbuild: the call is never resolved, so the package is neither bundled nor collected as an external to install in the deployed image. The deploy succeeds with zero diagnostics and the task fails at runtime with a module-not-found error, which can surface as something far more confusing when a library maps errors coarsely (a database driver loaded this way can look exactly like a connection failure). It also works fine in trigger dev because the local node_modules exists, making the production-only failure extra misleading.

Both deploy and dev builds now warn about this, pointing at the exact file and line, with a note showing the exact config that fixes it:

▲ [WARNING] "mssql" is loaded with createRequire() but won't be available in the deployed image, so loading it will fail at runtime. The bundler can't follow createRequire() calls, so "mssql" is neither bundled into your code nor installed in the image. [plugin create-require-collector]

    src/db.ts:12:14:
      12 │ const mssql = createRequire(import.meta.url)("mssql");
         ╵               ^

  To fix this, install "mssql" into the image by adding the additionalPackages build extension to your trigger.config.ts:

    import { additionalPackages } from "@trigger.dev/build/extensions/core";

    export default defineConfig({
      // ...
      build: {
        extensions: [additionalPackages({ packages: ["mssql"] })],
      },
    });

  Alternatively, replace the createRequire() call with a static import so the package is bundled. Docs: https://trigger.dev/docs/config/extensions/additionalPackages

In dev the message instead explains that the code works locally but deploys of it will fail, so the problem is caught while the code is being written rather than after a deploy.

How it works

An esbuild plugin scans the bundle's input files outside node_modules for string-literal specifiers passed to createRequire-created require functions: createRequire(...)("pkg"), const req = createRequire(...); req("pkg"), req.resolve("pkg"), aliased imports, namespace access, CJS destructuring, and dynamic import("node:module") bindings. Sources are parsed with @babel/parser (already in the dependency tree), so comments, strings, templates, regex literals and JSX can't confuse the scan; a file that fails to parse is skipped. Relative paths and node builtins never warn.

A usage only warns when the package will actually be missing from the image. On deploys the resolved manifest externals are the source of truth (extension-installed layers are already merged in when the warning runs); build.external alone deliberately does not suppress, because marking a package external installs nothing when nothing statically imports it. In dev, which predicts a future deploy, suppression additionally trusts what extensions declare they install, and stays silent entirely when that can't be determined (an extension hook throws, or an older @trigger.dev/build's additionalPackages predates the declaration hook), so dev never makes a false "deploys will fail" claim. additionalPackages declares its packages via a new diagnostics-only BuildExtension field, installedPackagesForTarget, which the bundler ignores: bundling output is unchanged for existing projects.

Detection is name-based, module-level, and deliberately per-file: computed specifiers, shadowed names, and require helpers imported from other files are not followed (those degrade to today's behavior, an unwarned runtime failure), and scanning is scoped to user code because bundled libraries legitimately use optional-require patterns that would drown real findings in noise. Packages named in build-layer install commands (RUN npm install ...) are suppressed individually.

Deploys also now surface esbuild's own bundle warnings for user files (for example require() with a non-literal argument), which were previously discarded on the deploy path; trigger dev already showed them.

Verification

Beyond the unit suite (54 tests, including real esbuild builds through the collector plugin), verified end to end against the hello-world reference project with the CLI linked to this branch:

  • trigger dev: a task loading mssql via createRequire produced the dev-phrased warning with the exact file:line code frame and the fix note during "Building local worker", and the local worker started normally. The project's real extensions (lightpanda, syncEnvVars, a custom inline extension) did not suppress it, and none of the project's other task files produced spurious warnings.
  • trigger deploy --dry-run, three passes: with the createRequire task it printed the deploy-phrased warning and still completed; after adding additionalPackages({ packages: ["mssql"] }) the warning disappeared and the build was clean; with the task removed and the config reverted, a pristine build produced zero warnings.

Source merge-base: 1d55693c0fc76279e7e8275fe41f959b65d5ea99
Source head: b1a98d535aeefe0aefb2fd7a323702e399bf7c26

@shipwright-agent

Copy link
Copy Markdown

⚠️ Shipwright · Approve with conditions

Recommendation: approve PR #3 with conditions · Tier T3
Checks: 0 total · 0 needing attention

Next step: an authorized approver must satisfy the approval condition.

Findings (9)

  • HIGH The 'installedPackagesForTarget' hook is documented as diagnostics-only and ignored by the bundler, but the implementation in 'additionalPackages' silently swallows parse errors wi · packages/build/src/extensions/core/additionalPackages.ts:25
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The 'extensionInstalledPackageMatchers' function marks the result incomplete when an 'additionalPackages' extension predates the hook, but this only suppresses dev-mode warnings. · packages/cli-v3/src/build/createRequireWarnings.ts:500
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The createRequire scanner silently skips files that fail to parse (parseWithFallbacks returns undefined and scanSourceForCreateRequire returns []). · packages/cli-v3/src/build/createRequireWarnings.ts:44
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The collector cache is keyed only by mtime and size. · packages/cli-v3/src/build/createRequireWarnings.ts:399
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The INSTALL_COMMAND_REGEX matches 'npm install' anywhere in a command string, including inside shell comments, heredocs, or quoted strings. · packages/cli-v3/src/build/createRequireWarnings.ts:548
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The 'packagesInstalledByCommands' function parses build-layer commands and treats any token matching an install command as a trusted package name. · packages/cli-v3/src/build/createRequireWarnings.ts:548
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The 'makeExternalRegexp' function is now exported and used to build regexes from extension-declared package names. · packages/cli-v3/src/build/externals.ts:522
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The 'CreateRequireCollector' reads and parses every bundle input file outside node_modules, including files that may be generated or user-controlled. · packages/cli-v3/src/build/createRequireWarnings.ts:384
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • …and 1 more findings in the check details.

Conditions

  • human approval required (T3): apply the approval label

Fireworks usage: 44,731 input · 1,360 output · 46,091 total tokens · $0.0107 · 24s · 0 fix iteration(s)

Open the Shipwright check for full evidence and the audit bundle. Use /shipwright rerun to verify again.

installedPackagesForTarget(target) {
if (target !== "deploy") {
return [];
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The 'installedPackagesForTarget' hook is documented as diagnostics-only and ignored by the bundler, but the implementation in 'additionalPackages' silently swallows parse errors wi

Impact: The 'installedPackagesForTarget' hook is documented as diagnostics-only and ignored by the bundler, but the implementation in 'additionalPackages' silently swallows parse errors with 'catch { continue; }'. A typo in a package specifier (e.g. 'npm:@scope/pkg@1.0.0' malformed) will silently drop the package from diagnostics, producing a false warning with no indication of why. The user has no way to know their extensi…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.


const usages: CreateRequireUsage[] = [];

for (const entry of scanned) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The 'extensionInstalledPackageMatchers' function marks the result incomplete when an 'additionalPackages' extension predates the hook, but this only suppresses dev-mode warnings.

Impact: The 'extensionInstalledPackageMatchers' function marks the result incomplete when an 'additionalPackages' extension predates the hook, but this only suppresses dev-mode warnings. In deploy mode, the warning still fires even though the extension may actually install the package. The comment acknowledges this is intentional, but a user upgrading an old config will see deploy warnings they cannot easily diagnose becaus…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

* `@babel/parser`, so comments, strings, templates, regex literals and JSX
* can never confuse the scan; a file that fails to parse is skipped
* (diagnostics must never fail a build). Binding tracking is name-based,
* module-level, and per-file: computed specifiers, shadowed names, and a

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The createRequire scanner silently skips files that fail to parse (parseWithFallbacks returns undefined and scanSourceForCreateRequire returns []).

Impact: The createRequire scanner silently skips files that fail to parse (parseWithFallbacks returns undefined and scanSourceForCreateRequire returns []). This means a syntax error or unsupported syntax in a user's source file will suppress the warning entirely, and the user will only discover the missing package at runtime in production. The feature's core promise is to warn before deploy, but parse failures create a sile…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

private _usages: CreateRequireUsage[] = [];
private _cache = new Map<string, CollectorCacheEntry>();
private _plugin: esbuild.Plugin | undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The collector cache is keyed only by mtime and size.

Impact: The collector cache is keyed only by mtime and size. A file modified within the same mtime granularity and with the same byte size will return stale specifiers, causing either missed warnings or warnings for code that no longer exists. This is a classic cache invalidation bug for dev rebuilds.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

let incomplete = false;

for (const buildExtension of config.build?.extensions ?? []) {
if (buildExtension.name === "externals") {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The INSTALL_COMMAND_REGEX matches 'npm install' anywhere in a command string, including inside shell comments, heredocs, or quoted strings.

Impact: The INSTALL_COMMAND_REGEX matches 'npm install' anywhere in a command string, including inside shell comments, heredocs, or quoted strings. A Dockerfile command like 'RUN echo "npm install fake-pkg"' would be parsed as installing fake-pkg, suppressing a legitimate warning. Conversely, complex shell constructs like 'npm install pkg1 && npm install pkg2' are handled, but 'npm install pkg1; npm install pkg2' is not, ca…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

let incomplete = false;

for (const buildExtension of config.build?.extensions ?? []) {
if (buildExtension.name === "externals") {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The 'packagesInstalledByCommands' function parses build-layer commands and treats any token matching an install command as a trusted package name.

Impact: The 'packagesInstalledByCommands' function parses build-layer commands and treats any token matching an install command as a trusted package name. An attacker who can influence the build commands (e.g. via a malicious extension or a compromised config) could inject a package name that suppresses warnings for a genuinely missing dependency, masking a runtime failure or a supply-chain gap. The regex has no anchoring t…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.


// Create the regex pattern
const pattern = `^${escapedPkg}(?:/[^'"]*)?$`;
export function makeExternalRegexp(packageName: string): RegExp {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The 'makeExternalRegexp' function is now exported and used to build regexes from extension-declared package names.

Impact: The 'makeExternalRegexp' function is now exported and used to build regexes from extension-declared package names. If an extension returns a package name containing regex metacharacters that are not properly escaped (e.g. a name with '[' or '('), the resulting regex could match unintended packages or fail to match the intended one. The escapeRegExp function is present, but the exported surface increases the risk of…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.


type CollectorCacheEntry = {
mtimeMs: number;
size: number;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The 'CreateRequireCollector' reads and parses every bundle input file outside node_modules, including files that may be generated or user-controlled.

Impact: The 'CreateRequireCollector' reads and parses every bundle input file outside node_modules, including files that may be generated or user-controlled. The parser is configured with 'errorRecovery: true' and multiple fallback plugin sets, but there is no size limit on files read. A malicious or accidental large file in the project could cause memory pressure during the build, especially with FILE_READ_CONCURRENCY=16 r…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

@@ -0,0 +1,729 @@
import { parse, ParserPlugin } from "@babel/parser";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · MEDIUM

The 'isCreateRequireCall' helper only recognizes 'createRequire' calls when the callee is an Identifier alias or a MemberExpression on a namespace import.

Impact: The 'isCreateRequireCall' helper only recognizes 'createRequire' calls when the callee is an Identifier alias or a MemberExpression on a namespace import. It does not recognize the common pattern 'const { createRequire } = require('module')' because 'collectVariableDeclarator' only records ObjectPattern properties when the initializer is a module builtin load, and 'isModuleBuiltinLoad' only accepts 'require('module'…

Suggested fix: Fix the review finding before release.

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.

1 participant