fix(dependency-audit): expand glob targets internally for cross-platform use - #61
Merged
Merged
Conversation
…orm use
A consumer invokes `dependency-audit ../../packages/*`, which relies on the
shell expanding the glob. POSIX shells do, but Windows cmd.exe/PowerShell pass
the literal pattern through, so the audit fails to find its targets.
Expand any positional containing glob magic (`* ? [ {`) via node:fs globSync
before auditing — already-expanded or magic-free targets (concrete paths,
specs, URLs) pass through untouched, and a pattern matching nothing is kept
verbatim so it still surfaces a clear "Target not found". Like a POSIX shell,
this does not de-duplicate.
Also add a Windows line to the node-compat CI job so the built CLI and its
tests run on Windows, with the smoke step auditing a quoted "packages/*" to
exercise the CLI's own expansion rather than the shell's.
Deploying mawesome with
|
| Latest commit: |
87b6de0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dce19f58.mawesome.pages.dev |
| Branch Preview URL: | https://fix-dependency-audit-glob-wi.mawesome.pages.dev |
…emory FS on Windows node:fs globSync is Node 22+, below the package's ^20.19 floor, so the prior import crashed the CLI on Node 20 (the whole suite failed). Replace it with a small dependency-free expander (src/glob-targets.ts) that matches a path-shaped glob's final segment against the base directory's immediate children — supporting `.`, `..`, and absolute bases (so `../../packages/*` still works), and never globbing a published spec or URL like `lodash@*`. A note marks it for replacement with node:fs glob once Node 20 support is dropped. Fold win32 separators to POSIX in the in-memory FileSystem so the browser/test adapter resolves paths the audit core joins with node:path on Windows — the five Windows failures were all this separator mismatch, unrelated to the glob change. Add hermetic unit tests for the expander; the CLI tests cover the end-to-end wiring. Addresses the Codex review (Node 20 crash, spec false-positive, unhandled glob errors, Windows backslash) and the Node 20 + Windows CI failures.
…n32 FS hack The in-memory FileSystem adapter is the browser path, where `node:path` is aliased to path-browserify (POSIX) and stays consistent with the POSIX-keyed tree. Driving the shared core over it on Windows-Node mixes win32 `node:path` joins with POSIX keys — a configuration that never ships (the CLI uses the real Node FS, which cli.test covers green on Windows). Guard those browser/in-memory auditPackage tests to non-Windows rather than papering over it with separator folding in the adapter (now reverted).
…hand-rolled matcher Delegate the filesystem walk and pattern matching to tinyglobby (a maintained, Node 20-compatible matcher) rather than a bespoke regex/readdir matcher. A base/tail split hands the literal base — which may be `..` or absolute, both of which tinyglobby rejects inside a pattern — to tinyglobby as `cwd`, so `../../packages/*` keeps working with no change to the Gutenberg call site. Multi-segment tails (e.g. `packages/*/package.json`) now expand too. Keeps the looksLikeSpec guard (so `lodash@*` still reaches pacote) and the verbatim-on-no-match behavior.
The expander normalizes patterns to `/` separators, so on Windows a matched target reads back with `/` even though the temp fixture path is `\`-spelled. Build the unit-test patterns and expectations from the `/`-form of the dir so the assertions hold on Windows (no-op on POSIX), matching how a real call passes forward slashes.
Tighten the glob-expansion comments and CLI/docs prose per the repo's brief-comments guideline, and correct the CLI reference (tinyglobby supports multi-segment tails, so the old "final segment only / non-recursive" note was both wordy and wrong).
…ob handling
From the Codex review:
- A root absolute glob (`/*`) now expands from `/` instead of cwd — the empty
leading segment was dropped, defaulting the base to `.`; the re-prefix also no
longer doubles the leading slash.
- Fold Windows `\` separators before the looksLikeSpec guard so a relative
backslash glob (`packages\*`) is recognized as a glob, not mistaken for a spec.
- Clarify (with a test) that the `firstMagic === -1` branch is reachable for a
brace group spanning `/` (e.g. `{a/b,c}`) — not dead code.
…b paths Close the two gaps the final Codex review flagged: a root-absolute `/*` that must expand from `/` (POSIX), and a Windows `\`-spelled glob that must fold to its `/` form (win32-only). Both exercise branches just fixed.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Expand path-shaped glob targets (e.g.
./packages/*,../../packages/*) inside thedependency-auditCLI instead of relying on the shell, and add a Windows line to the CI matrix.Why
A consumer in WordPress/gutenberg#79094 wires the tool up as
dependency-audit --collapse-root-cause ../../packages/*(run fromtools/validation/), which relies on the shell expanding the glob. POSIX shells do — but Windowscmd.exe/ PowerShell do not; they hand the literal pattern to the process, which then fails as "target not found". The reviewer flagged this directly.How
expandGlobTargets()expands a glob with tinyglobby (so the filesystem walk and matching are delegated to a maintained library, not hand-rolled). tinyglobby can't take a pattern that escapes itscwd(..) or is absolute, so we split the pattern at its first glob segment and hand the literal base to tinyglobby ascwd— which is what keeps../../packages/*working with no change to the Gutenberg call site. A published spec or URL (looksLikeSpec) is never globbed, solodash@*still reaches pacote; a pattern matching nothing is kept verbatim and surfaces as a clear "Target not found".^20.19Node floor (node:fs'sglobSyncis Node 22+).node-compatjob gains anosaxis: Linux keeps all three Node lines, Windows is added on one representative line (Node 24).harden-runneris gated to Linux; the test/smoke steps pinshell: bashso the.binshim and quoting are uniform. The smoke step audits a quoted"packages/*"— literal in every shell — so the CLI's own expansion is exercised against the built dist on a real Windows filesystem.Testing
?, dotfiles, multi-segment tails,..in the base), the spec/URL guard, no-dedup, and verbatim-on-no-match. The CLI tests cover the end-to-end wiring.../../packages/a11*fromtools/validation/expands and audits@wordpress/a11ycleanly.auditPackagetests are pinned to non-Windows: that adapter is the browser path (wherenode:pathis path-browserify / POSIX); driving the shared core over it on Windows-Node mixes win32 joins with POSIX keys, a config that never ships. The real Windows surface — the CLI over the real Node FS — is fully covered and green.Review
Addresses a Codex review pass: the original Node 20
globSynccrash, thelodash@*spec false-positive, unhandled glob errors, and Windows path handling.