Stop shipping the JavaScript build configuration in packages - #10403
Conversation
The Windows installer and the pip wheel were both shipping web/yarn.lock, web/package.json and the rest of the JavaScript build configuration into the installed tree, even though none of it is used at runtime: only the webpack output under pgadmin/static/js/generated is. Because node_modules is stripped, the lockfile that remains describes packages that are not present, so vulnerability scanners run against an installation report the whole build-time dependency tree, devDependencies included, as findings that cannot be acted on and will recur on every release no matter how current the versions are. The Linux, macOS and Docker builds already removed these files, so this brings Windows and pip into line with them, and adds webpack.* to the Linux and macOS lists so that all of the packagers now strip the same set. For pip the cleanup runs before syft, so the SBOM describes what actually ships. Verified by assembling a web tree with the cleanup applied and starting pgAdmin from it: /misc/ping and /browser/ both return 200 and every generated bundle is served.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughBuild scripts for Windows, Linux, macOS, and pip packages now remove JavaScript dependency metadata and build configuration files from staged or packaged web directories. ChangesBuild artifact cleanup
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to Packaged installations will omit JavaScript build configuration and dependency metadata while retaining generated runtime bundles. The supplied runtime checks show core application access continues to work, and no merge-blocking risk is identified. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 participants
The Windows installer and the pip wheel both ship
web/yarn.lock,web/package.jsonand the rest of the JavaScript build configuration into the installed tree, none of which is used at runtime, since only the webpack output underpgadmin/static/js/generatedis. Asnode_modulesis stripped, the lockfile that remains describes packages that are not actually present, so vulnerability scanners run against an installation report the whole build-time dependency tree,devDependenciesincluded. That produces findings nobody can act on, and they recur on every release however current the pinned versions happen to be. The report that prompted this flaggedpostcssandtaragainstC:\PostgreSQL\18\pgAdmin 4\web\yarn.lock, wherepostcssis only ever invoked bypostcss-loaderat bundle time andtaronly arrives transitively throughcacacheandnode-gyp.The Linux, macOS and Docker builds already removed these files, so this only brings Windows and pip into line with them:
Make.batremovednode_modulesand the.yarncache but leftyarn.lock,package.json,jest.config.js,babel.*,webpack.*and the lint/editor configuration behind.pkg/pip/build.shassembles the wheel fromgit ls-files, so it shipped the same files; the new cleanup runs beforesyft, so the SBOM now describes what actually ships rather than the build tree.webpack.*is added to the Linux and macOS lists, which Docker already dropped, so all four packagers now strip an identical set.pkg/src/build.shdeliberately keepsyarn.lock, since the source tarball genuinely is source.I verified this rather than assuming it: I assembled a
webtree with the cleanup applied, started pgAdmin from it, and both/misc/pingand/browser/return 200 with every generated bundle served, so nothing removed here is load-bearing at runtime. The Windows and macOS packagers I have not been able to exercise end to end locally, so those paths would benefit from a check on the build machines.Summary by CodeRabbit