Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bundled dependencies in monorepo contain ignored files when using --pack-command #4436

Open
zsstiers opened this issue Feb 28, 2024 · 0 comments
Labels
bug This issue is a bug. p2

Comments

@zsstiers
Copy link

zsstiers commented Feb 28, 2024

Describe the bug

Custom --pack-commands may not work correctly in monorepos with bundled dependencies. This is caused by the package manager losing important context when the copy is made for custom pack commands. This ultimately inflates the tarball by including files which should not be published.

In the project where I first encountered this a ~3MB tarball inflates to over 200MB as it starts including all source files, ignored files, test reports, and more.

This could be a security vulnerability for some users. Because this bug is caused by publishing files that are in .npmignore users may think they have their secrets ignored and unknowingly end up publishing them anyway.

Expected Behavior

Setting the pack command to "pnpm pack" produces results similar to manually running "pnpm pack".

Current Behavior

Using "pnpm pack" as the pack command causes bundled dependencies to contain the full source of the dependency rather than the output that should be packaged. I believe the reason for this is that since "pnpm pack" is now being run inside a copy of the package in a temp directory it is no longer able to identify that those packages belong to the same workspace and so need to recursively be packed, not just copied in.

I'm presently working around this issue by using --pack-command="cd \"$PWD\" && pnpm pack --pack-destination \"$(mktemp -d)\"" to ignore the copy that was made and pack in the original location instead.

Reproduction Steps

Run the following in an empty directory. Expects that npm, pnpm, and standard utils are available.

pnpm init
printf "packages:\n  - 'packages/*'" > pnpm-workspace.yaml
mkdir -p packages/foo packages/bar
(cd packages/foo && pnpm init)
echo "/ignored" > packages/foo/.npmignore
touch packages/foo/ignored
(cd packages/bar && pnpm init)
(cd packages/bar && pnpm add foo --workspace && pnpm add -D jsii jsii-pacmak && npm pkg set 'bundledDependencies[]'=foo)
(cd packages/bar && npm pkg set --force 'author.name'=unused 'author.email'=unused 'repository.type'=git 'repository.url'=unused 'types'='lib/index.d.ts')
(cd packages/bar && npm pkg set --json 'jsii'='{"outdir":"dist","targets":{},"tsc":{"outDir":"lib","rootDir":"src"}}')
mkdir -p packages/bar/src
echo "export class Bar {}" > packages/bar/src/index.ts
(cd packages/bar && npx jsii && npx jsii-pacmak --pack-command="pnpm pack")
tar -tf 'packages/bar/dist/js/[email protected]'

when complete observe that files that should not be present, such as "package/node_modules/foo/ignored", are present in the tarball. If you replace the text pnpm pack with cd \"$PWD\" && pnpm pack --pack-destination \"$(mktemp -d)\" then the issue will go away, although a wasteful copy will still occur.

The above was executed on both AL2 and an M1 Mac.

Possible Solution

As can be observed by the custom command which cds back to $PWD to avoid using the copy, not performing the copy present on https://github.com/aws/jsii/blob/main/packages/jsii-pacmak/lib/packaging.ts#L71-L72 can be used to solve the problem. Simply removing the copying though brings back the parallelism issue mentioned in the comment.

My preference as a consumer of JSII would be to not need to know I need to specify a custom pack-command for pnpm at all though. The most popular package managers could be detected via lock files and independently correctly handled. Sadly, this doesn't perfectly scale for every possible case as more package managers are created though.

Additional Information/Context

No response

SDK version used

1.93.0

Environment details (OS name and version, etc.)

Amazon AL2

@zsstiers zsstiers added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 28, 2024
@mrgrain mrgrain added p2 and removed needs-triage This issue or PR still needs to be triaged. labels Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p2
Projects
None yet
Development

No branches or pull requests

2 participants