fix: ship a workspaces monorepo function's own dependencies when building in source

[bnusunny]

Issue #, if available: Fixes #933

Stacked on #931. GitHub cannot base an upstream PR on a fork branch, so this PR targets develop and its first commit (74d8888) is #931's. Review only the last two commits — b9cd051 and bb7368b. If #931 merges first, this diff shrinks to exactly those two. They share SubprocessNpm, the install_dir refactor and the experimental flag with #931, and the fixture's lockfile pins versions below what its manifests allow, so the assertions only mean what they say once the lockfile is honoured.

Description of changes

nodejs_npm with --build-in-source produces an artifact with no dependencies at all for an npm workspaces monorepo, and reports success. npm hoists a workspace member's dependencies to the monorepo root, so nothing is installed beside the function; the artifacts link looks for node_modules next to the function, finds none, and silently skips. The function then fails at runtime with Cannot find module.

Opt-in while it rolls out. Both changes are behind experimentalNodejsMonorepo - sam-cli's
ExperimentalFlag.NodejsMonorepo, the same flag #931 uses. Without it npm is not asked where its project
root is and the artifacts link stays exactly where it is today, which for a monorepo means this bug is
still there: that is deliberate, so a release can carry the fix while users opt into it.
test_without_the_flag_a_monorepo_keeps_the_plain_link_and_npm_is_not_asked pins that side.

Two changes, one per commit.

cabe7ee — link the artifacts to wherever npm actually installed. SubprocessNpm.resolve_project_root asks npm itself (npm prefix, cached per directory), and only a project root outside the install directory redirects the link. npm answers with the install directory itself for anything that is not a workspace member, so every existing path — including the external-manifest one, which links through the source tree — is left exactly as it was.

070a021 — ship only the function's own dependencies. Linking the whole hoisted root would put every sibling function's dependencies into every artifact. NodejsNpmLinkDependencyClosureAction links just the packages the function resolves, each under its own name:

case	behaviour
a package the function depends on	linked into the artifacts under its own name, scope included
a workspace sibling (@mono/shared)	linked under its package name, resolved to its source directory
a nested copy npm kept because a dependency pins a conflicting version	left nested — hoisting it would shadow the top-level version for every other caller
a sibling function's dependency	not linked
npm cannot answer	the whole installed tree is linked rather than guessing: an over-complete node_modules still runs
a package npm named that has no readable manifest	the build fails loudly, rather than placing it where node will not find it

Both path comparisons go through os.path.normcase. They compare npm's stdout against a build-computed path, and on Windows two spellings that differ only in case — a drive letter included — name the same directory; reading them as different roots would redirect the link for a project that is not a workspace member, or hoist a nested copy. It is a no-op off Windows.

The esbuild workflow needs none of this: it bundles what each entry point imports into the output file, resolving through the hoisted root, so its artifacts were already correct.

New fixture manifests and lockfiles under the nodejs_npm integration testdata/ directory are test data, not a dependency change to this package.

Description of how you validated changes

The new integration test fails without the fix. With this branch's tests kept and only the three production files reverted to #931, test_build_in_source_in_workspaces_monorepo_links_the_hoisted_dependencies fails on all five supported runtimes with AssertionError: False is not true : the artifacts have no node_modules at all — the #933 symptom exactly.

The fixture is a workspaces monorepo with two functions on disjoint dependencies (minimal-request-promise, ms), a shared workspace package both depend on, and a version conflict the shared package pins so npm keeps a nested copy. Against the real npm, across every supported runtime, the test asserts each function's artifacts carry its own dependency and the shared package but not its sibling's, that the nested conflicting copy stays nested, and that node -e "require('./included.js')" succeeds from each artifacts directory — so the artifact is covered, not just the installed tree.

Unit tests cover the closure selection (test_links_only_this_function_s_dependencies, test_links_a_workspace_dependency_under_its_own_name, test_leaves_a_nested_copy_inside_the_dependency_that_pins_it), the no-answer fallback (test_links_the_whole_installed_tree_when_npm_cannot_be_asked), the missing-manifest failure (test_fails_loudly_when_a_reported_package_has_no_manifest), the workflow wiring for both the monorepo and the unchanged non-monorepo path, and npm prefix resolution including its per-directory caching and that a failure is cached too.

• 855 unit tests pass; black --check clean.
• nodejs + esbuild integration workflows: 295 passed on npm 11.19.0, Linux. Also run on npm 10.9.9 — 295 passed — because npm prefix and hoisting are npm-version dependent.
• Windows is untested outside CI. The link steps go through the existing create_symlink_or_copy, which falls back to a copy when os.symlink is not permitted; the windows-latest lanes on this PR are the first run.

Checklist

• Review the generative AI contribution guidelines

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[aws-sam-tooling-bot]

Code Review Results

Reviewed: 587257c..070a021
Files: 34 (6 source/test-logic files reviewed in depth; JSON/JS fixtures skimmed)
Comments: 3

[aws-sam-tooling-bot]

[BUG] Two closure entries with the same package name silently collapse into one link, and which one wins depends on npm's output order.

_outermost_packages filters a nested copy only when it sits inside another candidate. Both project_root and install_dir are removed from candidates, so a nested copy under the function's own node_modules is never recognised as nested:

closure = [
  project_root,                                  # excluded
  install_dir,                                   # excluded
  project_root/node_modules/lodash,              # lodash@1, hoisted for pkg-a
  install_dir/node_modules/lodash,               # lodash@2, nested because fn pins it
]

Neither lodash path is a prefix of the other, and neither is inside a candidate, so both are returned and both resolve to link_path = <artifacts>/node_modules/lodash. The second utils.create_symlink_or_copy call hits the early return in create_symlink_or_copy (destination.exists() and destination.is_symlink()) and returns without a warning, so one version is dropped. On a host where symlinks fall back to copying, the second call instead raises FileExistsError, is caught, and copytree merges two different versions into the same directory. This is the version-conflict case in a monorepo, which the unit tests only cover for a copy nested inside a sibling package (test_leaves_a_nested_copy_inside_the_dependency_that_pins_it), not one nested under install_dir.

Suggest treating install_dir as a containing directory for the nesting check (a copy under install_dir/node_modules is the function's own and should win at top level) and failing or logging loudly on a genuine name collision rather than letting it resolve by chance.

[aws-sam-tooling-bot]

[SECURITY] name is read from a third-party dependency's package.json and joined straight into a filesystem path:

link_path = os.path.join(destination, *name.split("/"))
os.makedirs(os.path.dirname(link_path), exist_ok=True)
utils.create_symlink_or_copy(package_dir, link_path)

A manifest with "name": "../../../evil" escapes the artifacts directory, and os.makedirs will create the intermediate directories to get there. The registry rejects such names, but nothing here validates what is actually on disk (a vendored or locally-linked package, or a tampered cache). Worth checking that the resolved link_path stays under destination before creating anything:

if os.path.commonpath([os.path.abspath(destination), os.path.abspath(link_path)]) != os.path.abspath(destination):
    raise ActionFailedError(f"Package name {name!r} in {package_dir} does not resolve inside node_modules")

[aws-sam-tooling-bot]

[BUG] The closure is None fallback links project_root/node_modules without checking that it exists. os.symlink succeeds against a missing target, so the artifact gets a dangling node_modules symlink — worse than the no-op it replaces. LinkSinglePathAction, which this action supersedes on this path, guards for exactly this:

if not source_path.exists():
    LOG.debug("Source path %s does not exist, skipping generating symlink", source_path)
    return

Reachable when npm ls exits non-zero on a tree that also produced no root node_modules. Adding the same existence check (and skipping) keeps the failure mode at "no dependencies" rather than "broken link".

[aws-sam-tooling-bot]

Code Review Results

Reviewed: 587257c..bb7368b
Files: 36 (8 logic files reviewed in depth; JSON/JS fixtures skimmed)
Comments: 4

[aws-sam-tooling-bot]

[BUG] Re-raising: a nested copy under the function's own node_modules is never recognised as nested, so two closure entries with the same package name collapse into one link and which one wins depends on npm's output order.

install_dir is removed from candidates, so is_nested_in_another can never match it as the containing directory:

closure = [
  project_root,                                 # excluded
  install_dir,                                  # excluded
  install_dir/node_modules/lodash,               # lodash@1, pinned by the function
  project_root/node_modules/c,                   # depends on lodash@2
  project_root/node_modules/lodash,              # lodash@2, hoisted
]

Both lodash paths survive _outermost_packages and both resolve to artifacts/node_modules/lodash. create_symlink_or_copy returns early when the destination already exists as a symlink, so the first one npm happened to print wins and the other version is silently dropped — either the function or c gets the wrong lodash. test_leaves_a_nested_copy_inside_the_dependency_that_pins_it only covers the nested-under-another-dependency case, which does work.

The nesting test needs to consider install_dir (and project_root) as containers even though they are not link candidates, e.g. keep a separate containers = candidates + [project_root, install_dir] list and test is_nested_in_another against that. A same-name collision that survives should at minimum be logged rather than resolved by output order.

[aws-sam-tooling-bot]

[SECURITY] Re-raising: name comes from a third-party dependency's package.json and is joined straight into a filesystem path, so a hostile manifest can write outside the artifacts directory.

link_path = os.path.join(destination, *name.split("/"))
os.makedirs(os.path.dirname(link_path), exist_ok=True)
utils.create_symlink_or_copy(package_dir, link_path)

"name": "../../../evil" yields ["..", "..", "..", "evil"], and os.makedirs will happily create the traversed intermediates before the symlink (or, on the copy fallback, a full copytree) lands outside artifacts_dir. An absolute name ("/tmp/x" → os.path.join discards destination) is worse. Nothing between npm ls and here validates the name.

Validate before use: reject a name that is not exactly pkg or @scope/pkg with no path separators, .., or drive/root prefix in either segment, and assert os.path.commonpath([os.path.realpath(destination), os.path.realpath(link_path)]) == os.path.realpath(destination).

[aws-sam-tooling-bot]

[BUG] Re-raising: the closure is None fallback links project_root/node_modules without checking it exists, producing a dangling symlink in the artifacts.

create_symlink_or_copy goes straight to os.symlink(Path(source).absolute(), ...), which succeeds against a missing target on POSIX, so the artifact ends up with a node_modules symlink that points nowhere — a worse outcome than the no-op it replaces, since downstream CopyDependenciesAction/copytree will then follow or reproduce a broken link. LinkSinglePathAction, which this action supersedes on this path, guards for exactly this:

if not source_path.exists():
    LOG.debug("Source path %s does not exist, skipping generating symlink", source_path)
    return

Add the same existence check before linking (and log at debug/warning so the empty-dependency case is visible). test_links_the_whole_installed_tree_when_npm_cannot_be_asked always creates the tree first, so it does not cover this.

[aws-sam-tooling-bot]

[BUG] An aliased dependency is linked under the wrong name. npm supports "lodash4": "npm:lodash@^4.0.0", which installs at node_modules/lodash4 while the on-disk manifest still says "name": "lodash". npm ls --parseable only prints the path, so taking the name from the manifest loses the alias: the artifact gets node_modules/lodash and require("lodash4") fails at runtime with Cannot find module — the same failure this PR is fixing. Two aliases of the same package additionally collide on one destination (see comment 1).

The path already carries the correct name for anything npm installed: the segments after the last node_modules component (lodash4, or @scope/name for a scoped install). The manifest name is only needed for the case the docstring calls out — a workspace dependency reported as its own source directory, which is not under node_modules. Preferring the path-derived name and falling back to the manifest only outside node_modules fixes the alias case and removes the untrusted-name path traversal in comment 2 at the same time.