-
Notifications
You must be signed in to change notification settings - Fork 240
build(desktop): stop shipping the renderer's dependency tree twice #3148
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
Changes from all commits
3cff8ec
cae8aa3
ca4fcef
bf03b39
573d763
dd6d7b6
1ba8a62
b05576d
b9a7833
e7a938b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| // Records which npm packages the renderer bundle actually contains. | ||
| // | ||
| // The module graph sees direct and deep JS imports; the emitted assets carry | ||
| // the rest of the chain, which is how a package reached only through CSS | ||
| // (Fontsource, via its `.woff2` files) still lands here. What neither sees is | ||
| // a stylesheet that imports a package of pure rules: Vite inlines a CSS | ||
| // `@import` at transform time, so the imported file never becomes a module, | ||
| // and a package whose CSS emits no `url()` asset leaves no trace in this | ||
| // record. `validateFirstPartyCssImports` in the notice generator covers that | ||
| // case by reading the stylesheets themselves — stated here because the gap is | ||
| // invisible from this file, and the check that closes it lives elsewhere. | ||
| // | ||
| // The JSON ships inside `dist-renderer`, which lets the release verifier judge | ||
| // the packaged artifact by the artifact's own record. | ||
| export function bundledNpmPackagesPlugin() { | ||
| return { | ||
| name: 'maka-bundled-npm-packages', | ||
| apply: 'build', | ||
| generateBundle(_options, bundle) { | ||
| const packages = new Set(); | ||
| const collect = (id) => { | ||
| // Virtual modules (\0-prefixed) are build-tool internals, not packages. | ||
| if (typeof id !== 'string' || id.startsWith('\0')) return; | ||
| // The last node_modules segment names the package that owns the file, | ||
| // even for nested installs (node_modules/a/node_modules/b/...). | ||
| const matches = [...id.matchAll(/[\\/]node_modules[\\/]((?:@[^\\/]+[\\/])?[^\\/]+)(?=[\\/])/g)]; | ||
| if (matches.length === 0) return; | ||
| const name = matches[matches.length - 1][1].replaceAll('\\', '/'); | ||
| if (name === '.vite') return; | ||
| packages.add(name); | ||
| }; | ||
| for (const id of this.getModuleIds()) collect(id); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Collect the CSS pipeline's dependencies too, or weaken the claim in this file's header.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for adding the first-party stylesheet scan — that covers the current production imports. One small P3 parser/test blind spot remains: |
||
| // CSS `@import` chains are inlined by the CSS pipeline and never become | ||
| // rollup modules, but the files they pull in (fonts, images) are emitted | ||
| // as assets that remember their source paths — that is how a package | ||
| // reachable only through CSS (Fontsource) still lands in this record. | ||
| for (const output of Object.values(bundle)) { | ||
| if (output.type !== 'asset') continue; | ||
| for (const original of output.originalFileNames ?? []) collect(original); | ||
| } | ||
| this.emitFile({ | ||
| type: 'asset', | ||
| fileName: 'bundled-npm-packages.json', | ||
| source: `${JSON.stringify([...packages].sort(), null, 2)}\n`, | ||
| }); | ||
| }, | ||
| }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2 — Shipped renderer code falls out of the release vulnerability audit. The workflows still use
npm audit --omit=dev; after this move, React and the other declared renderer roots remain indist-rendererbut are excluded from that audit solely because npm calls them dev dependencies. Extend the existing shipped-artifact graph to an auditable renderer closure (or run an equivalent dedicated bundled-graph audit) so the security boundary matches what is released.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rechecked on exact head
9e74bcc95. The dedicated audit is a good step, but this P2 remains: it still derives the renderer closure from the hand-maintainedmaka.rendererBundledDependencieslist, while the dependency-audit workflow does not build/read Vite's actualdist-renderer/bundled-npm-packages.json. A newly imported renderer package omitted from the list can therefore ship without this security lane auditing it; only the later release artifact verifier detects the drift. Please make the audit consume the actual bundled graph, or build and validate that graph in the audit workflow.中文说明
当前 head 已新增专用 audit,但它仍以手工 rendererBundledDependencies 为权威,而 dependency-audit workflow 不生成或读取 Vite 的真实 bundled graph。新增但漏列的 renderer import 仍会绕过安全审计,只在更晚的 release verifier 才被发现,因此这个 P2 仍有效。