fix(zip)!: make includeSources/excludeSources follow standard allowlist behavior - #2114
Conversation
…-dev#1623) Co-authored-by: Aaron <aaronklinker1@gmail.com>
Co-authored-by: Patryk Kuniczak <p.kuniczak@gmail.com>
When includeSources patterns are provided, only files matching those patterns should be included in the sources ZIP. Previously, includeSources patterns were added to the default **/* glob, which could leak sensitive files that weren't explicitly excluded. Changes per maintainer review: - Move default ['**/*'] into resolveZipConfig - Pass excludeSources directly to glob's ignore option - Remove hardcoded node_modules glob (already in excludeSources default) - Simplify zipDir logic BREAKING CHANGE: If you were using includeSources to add specific files to the default set (e.g., hidden files), you now need to include all files you want in the ZIP. Fixes wxt-dev#2059
There was a problem hiding this comment.
While reviewing, I remembered some past issues related to zipping sources that we need to support:
TLDR: We need to support including hidden files and files in hidden directories. I think this requirement was why the logic was so complex.
Right now, I don't think this PR supports adding hidden files at all.
I ran some tests to refresh my knowledge of fast-glob, verifying the three cases we care about (regular files, hidden files, regular files in hidden directories):
Details
import { glob, Options } from "fast-glob";
import fs from "node:fs/promises";
import { inspect } from "node:util";
const test = async (sources: string[], options: Options) => {
const res = await glob(sources, options);
console.log(
`
───
glob(
${inspect(sources, { colors: true })},
${inspect(options, { colors: true })}
)
= ${inspect(res, { colors: true })}`,
);
};
const allFiles = await fs.readdir(".", {
recursive: true,
});
console.log("All files (minus node_modules):");
console.log(
allFiles.filter((file: string) => !file.startsWith("node_modules")),
);
await test(["**/*"], { ignore: ["**/node_modules/**"] });
await test(["**/.*"], { ignore: ["**/node_modules/**"] });
await test(["**/.*/**/*"], { ignore: ["**/node_modules/**"] });and the output:
All files (minus node_modules):
[ ".wxt", "bun.lock", "package.json", ".env", "index.ts", ".wxt/test.txt" ]
───
glob(
[ '**/*' ],
{ ignore: [ '**/node_modules/**' ] }
)
= [ 'bun.lock', 'package.json', 'index.ts' ]
───
glob(
[ '**/.*' ],
{ ignore: [ '**/node_modules/**' ] }
)
= [ '.env' ]
───
glob(
[ '**/.*/**/*' ],
{ ignore: [ '**/node_modules/**' ] }
)
= []
But it seems like fast-glob doesn't support including files from inside hidden directories (I did try the dot: true option as well, but it didn't make a difference)...
So I tried glob instead:
Details
import { glob, GlobOptions } from "glob";
import fs from "node:fs/promises";
import { inspect } from "node:util";
const test = async (sources: string[], options: GlobOptions) => {
const res = await glob(sources, options);
console.log(
`
───
glob(
${inspect(sources, { colors: true })},
${inspect(options, { colors: true })}
)
= ${inspect(res, { colors: true })}`,
);
};
const allFiles = await fs.readdir(".", {
recursive: true,
});
console.log("All files (minus node_modules):");
console.log(
allFiles.filter((file: string) => !file.startsWith("node_modules")),
);
await test(["**/*"], { ignore: ["**/node_modules/**"] });
await test(["**/.*"], { ignore: ["**/node_modules/**"] });
await test(["**/.*/**/*"], { ignore: ["**/node_modules/**"] });
await test(["**/.*/**/*"], { ignore: ["**/node_modules/**"], nodir: true });All files (minus node_modules):
[ ".wxt", "bun.lock", "package.json", ".env", "index.ts", ".wxt/test.txt" ]
───
glob(
[ '**/*' ],
{ ignore: [ '**/node_modules/**' ] }
)
= [ 'index.ts', 'package.json', 'bun.lock' ]
───
glob(
[ '**/.*' ],
{ ignore: [ '**/node_modules/**' ] }
)
= [ '.env', '.wxt' ]
───
glob(
[ '**/.*/**/*' ],
{ ignore: [ '**/node_modules/**' ] }
)
= [ '.wxt/test.txt' ]
───
glob(
[ '**/.*/**/*' ],
{ ignore: [ '**/node_modules/**' ], nodir: true }
)
= []
It works, but I can't get it to return just files. It does support returning with file-types, so we could filter out directories that way.
Unforunate since glob is 10x as large...
- https://pkg-size.dev/fast-glob - 515KB / 18 packages
- https://pkg-size.dev/glob - 5MB / 9 packages
Maybe there's another glob library that's lighter-weight and would allow devs to include hidden files and files in hidden directories?
Otherwise I think we have to switch from fast-glob to glob.
| import JSZip from 'jszip'; | ||
| import glob from 'fast-glob'; | ||
| import { normalizePath } from './utils/paths'; | ||
| import { minimatchMultiple } from './utils/minimatch-multiple'; |
There was a problem hiding this comment.
We can delete this file and it's test completely, this was the only place they were used.
|
Can you also update the docs, adding a section about this change to the docs here: wxt/docs/guide/resources/upgrading.md Line 66 in 19c1d0a Maybe wait to do that until all the changes are finalized through. |
|
I'm not making any claims about the quality of their work, but I wanted to let you know that @kaigritun is a fully-autonomous non-human actor |
|
@kaigritun Are you want to go on? |
|
@kaigritun One more poke :) |
includeSources/excludeSources follow standard allowlist behavior
aklinker1
left a comment
There was a problem hiding this comment.
I had to make some changes around how hidden files are included, but I'm happy with the result. I imagine for wxt 1.0, we may need to provide the full options for tinyglobby, not just include, exclude, and dot. But this is good enough to get feedback on.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## major #2114 +/- ##
==========================================
- Coverage 79.48% 79.26% -0.22%
==========================================
Files 132 131 -1
Lines 3982 3970 -12
Branches 908 905 -3
==========================================
- Hits 3165 3147 -18
- Misses 728 731 +3
- Partials 89 92 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thanks for helping make WXT better! |
…owlist behavior (#2114) Co-authored-by: Eli <88557639+lishaduck@users.noreply.github.com> Co-authored-by: ТΞNSΛI <tensai@gmx.net> Co-authored-by: Aaron <aaronklinker1@gmail.com> Co-authored-by: Patryk Kuniczak <p.kuniczak@gmail.com> Co-authored-by: Kai Gritun <kai@kaigritun.com>
…owlist behavior (#2114) Co-authored-by: Eli <88557639+lishaduck@users.noreply.github.com> Co-authored-by: ТΞNSΛI <tensai@gmx.net> Co-authored-by: Aaron <aaronklinker1@gmail.com> Co-authored-by: Patryk Kuniczak <p.kuniczak@gmail.com> Co-authored-by: Kai Gritun <kai@kaigritun.com>
wxt 0.21 changed zip.includeSources from an additive, exclude-overriding list to a plain allowlist (wxt-dev/wxt#2114). Our config listed only entrypoints/offscreen/index.html there (to force-include the Chrome-only entrypoint that wxt auto-excludes as skipped on Firefox), so the AMO sources zip ended up completely empty. Drop the includeSources override to get the full-sources default back, and re-add the offscreen entrypoint in the zip:sources:done hook via jszip, since wxt no longer offers a way to un-exclude a skipped entrypoint. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When includeSources patterns are provided, only files matching those patterns should be included in the sources ZIP. Previously, includeSources patterns were added to the default
**/*glob, which could leak sensitive files that weren't explicitly excluded.Changes per maintainer review (replaces #2111):
['**/*']intoresolveZipConfigignoreoptionnode_modulesglob (already in excludeSources default)zipDirlogicdotSourcesoption to allow configuringtinyglobby'sdotoption, allowing for zipping hidden filesBREAKING CHANGE: Sources zip generation behavior has changed.
Fixes #2059
Supersedes #2111