glob: validate continuation bytes when stepping over directory entry names

[robobun]

Problem

• The glob matcher (src/glob/matcher.rs) steps ?, * and [...] over path bytes by the length the UTF-8 lead byte promises and never checks the following bytes. A Linux file name need not be valid UTF-8.
• So ? and * swallow the next byte, a / included, or step past the end. bun pm pack packs caf\xe9.pem and secret/key\xc3 that .npmignore (*.pem, secret/*) excludes. new Bun.Glob("t*.js") misses t\xe4\xb8.js.

Fix

• bun_core::strings::utf8_codepoint_with_fffd exposes the strict decoder that turns these names into JS strings. The matcher steps by its len: one U+FFFD per ill-formed piece, never past the end.
• So scan(p) equals scan("*") filtered by Glob.match(p) for wildcards and classes. The test asserts it.
• Verified: test/js/bun/glob/scan.test.ts (11 new cases, stock bun fails 9), match.test.ts, the Bun.Archive glob tests, the bun pm pack ignore tests.

Background

• A Linux file name is any bytes without / or NUL. Latin-1 archives, Windows zips and Samba shares produce non-UTF-8 names.
• Considered a private decode table in the glob crate: it duplicated the shared decoder and disagreed with it on ED A0 80.

Downsides

• A valid non-ASCII name pays a decoder call per non-ASCII character under ? or *: +26.8% instructions per Glob.match() over six non-ASCII pairs (+8% for report-café-résumé.txt, +51% for a long Japanese name). ASCII is tested first and never calls: -6.4%.
• A literal U+FFFD in a pattern still compares bytes: t�* misses an ill-formed name that t[�]* matches.
• A raw ED A0 80 in a name is three characters for ? now, one before.

Notes

Repro for the pack and archive cases (Linux, bun 1.4.3-canary.1+367d939d9 against this branch):

mkdir secret; echo '{"name":"p","version":"1.0.0"}' > package.json; printf '*.pem\nsecret/*\n' > .npmignore
python3 -c "
for n in (b'index.js', b'ok.pem', b'caf\xe9.pem', b'secret/key', b'secret/key\xc3', b'report-caf\xe9.txt', b'report-ok.txt'): open(n, 'w').close()"
bun pm pack --dry-run

Stock bun lists caf\xe9.pem and secret/key\xc3 as packed. This branch packs neither. new Bun.Glob("report-*.txt").scanSync(".") finds 1 of 2 on stock bun, 2 of 2 here. With a tar that holds pub/ok.txt, pub/x\xc3/deep.txt, secret/key and secret/key\xc3, new Bun.Archive(tar).files("pub/*") also returns pub/x\xc3/deep.txt on stock bun, and files(["**", "!secret/*"]) keeps secret/key\xc3. Here pub/* returns only pub/ok.txt and no secret/ member passes.

Instruction counts. Release builds (bun run build:release) of this branch, and of this branch with src/glob/matcher.rs from main, so the matcher is the only difference. perf and valgrind are not available in the build container and perf_event_open is not permitted. A ptrace tracer single-steps the main thread between two marker signals instead. Each pair runs 24 and then 48 Glob.match() calls with BUN_JSC_useJIT=0. The table shows (count(48) - count(24)) / 24, instructions per call. Two runs of each binary give identical counts in all 24 regions.

pattern	string	main	this branch
**/*.test.ts	test/js/bun/glob/scan.test.ts	3292	3165	-3.9%
report-*.txt	report-quarterly-2024.txt	2130	2072	-2.7%
src/**/*.{ts,tsx}	src/runtime/server/ServerWebSocket.ts	4034	3851	-4.5%
????-??-??.log	2024-09-08.log	1000	1001	+0.1%
*[0-9].md	changelog-v12.md	2999	2412	-19.6%
*.js	a-rather-long-file-name-that-is-not-javascript.css	4581	4379	-4.4%
ASCII sum		18036	16880	-6.4%
report-*.txt	report-café-résumé.txt	2528	2731	+8.0%
*文*.txt	中文字中文字中文字.txt	2183	2945	+34.9%
**/*.md	ドキュメント/設計/概要.md	2941	3992	+35.7%
???.txt	日本語.txt	1240	1508	+21.6%
*[é]*.pem	naïve-café-déjà-vu.pem	4439	4702	+5.9%
*.js	ファイル名がとても長いけれどジャバスクリプトではない.css	4238	6404	+51.1%
non-ASCII sum		17569	22282	+26.8%

.text is 1,280 bytes smaller than with main's matcher (80,677,713 against 80,678,993).

The non-ASCII cost can be recovered: a length-only step that recognises a well-formed sequence inline and asks the decoder for anything else measures +5.3% on the non-ASCII sum and -5.5% on the ASCII sum in the same setup. It is not in this PR.

Limits. A literal U+FFFD in a pattern compares bytes (EF BF BD), so it does not match an ill-formed piece on disk. A component without a wildcard never reaches the matcher (the walker opens it by its bytes), so a change to the literal arm alone would make t�* match a name that t�x still misses.

Self-review. The first revision carried a private Unicode Table 3-7 transcription in the glob crate that kept WTF-8 surrogates (ED A0..BF) as one 3-byte unit. The review raised that bun_core already has the strict decoder and that the surrogate row disagreed with how the returned names decode. Both addressed: the helper is in bun_core::strings over the existing decoder, and the test pins the ED A0 80 row.

Tests. The #[test] table for utf8_codepoint_with_fffd is compile-checked only: bun_core is not in the crate list of the miri job, and cargo test -p bun_core does not link standalone. Its rows are exercised through the scan test. Six glob.match > ... recursive search cases in scan.test.ts scan the whole repository with a hardcoded 30 s timeout. They time out in this container under debug+ASAN with and without this diff.

Counts after the fix for a directory that holds t\xc3x, t\xe4\xb8.js, t\xc3\xa9\xc3, t😀js and tx\x80: * 5, t* 5, t? 0, t?? 3, t??? 1 (t😀js), t*.js 1, t*s 2. fs.globSync differs only on t??? (0) because its ? matches one UTF-16 code unit and the emoji is two.

---

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/glob/scan.test.ts

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 0f022aff-87d5-44df-9254-2ed2380af0df

📥 Commits

Reviewing files that changed from the base of the PR and between 4b02e10 and bd23de9.

📒 Files selected for processing (4)

• src/bun_core/string/immutable.rs
• src/bun_core/string/immutable/unicode.rs
• src/glob/matcher.rs
• test/js/bun/glob/scan.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

Walkthrough

Glob matching now decodes path and pattern characters as UTF-8, using U+FFFD for malformed sequences. A public decoder helper and Linux tests for malformed UTF-8 filenames were added.

Changes

UTF-8 replacement decoder

Layer / File(s)	Summary
UTF-8 replacement decoder src/bun_core/string/immutable/unicode.rs, src/bun_core/string/immutable.rs	Adds and re-exports a decoder that returns a code point and consumed length. Tests cover valid input, malformed sequences, and truncation.

Glob matching with malformed UTF-8

Layer / File(s)	Summary
Glob matching and scan coverage src/glob/matcher.rs, test/js/bun/glob/scan.test.ts	Glob path and pattern characters use UTF-8 decoding with U+FFFD replacement. Linux tests check wildcard, character-class, and extension patterns against scan results and Glob.match.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to bd23d

This change makes glob wildcard and character-class matching handle invalid UTF-8 filenames consistently with how scan decodes them. No merge-blocking risk was identified.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the main change: validating continuation bytes while glob matching steps through directory entry names.
Description check	✅ Passed	The description explains the problem, fix, behavior changes, tradeoffs, and verification results. It does not use the template headings exactly, but it provides the required information through the Fi…

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 10:06 AM PT - Oct 1st, 2026}

✅ @robobun, your commit 39c3740329a663e2c3dd3f608ccb6a616b4824ce passed in Build #122448! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 41985

That installs a local version of the PR into your bun-41985 executable, so you can run:

bun-41985 --bun

[claude]

I reviewed this and didn't find any bugs. The fix looks correct and well-tested, but since it changes how the matcher steps over ill-formed bytes (WTF-8 surrogate handling now differs from the old decode_wtf8_rune_t path — ED A0 80 becomes three units instead of one), a human sign-off on that semantic choice would be worthwhile.

What was reviewed:

• Traced convert_utf8_bytes_into_utf16 with zero-padded tails — len is always in 1..=bytes.len(), so rune_len_at cannot overshoot the slice.
• Confirmed all four path-side stepping sites are updated; the remaining wtf8_byte_sequence_length in matcher.rs:383 and GlobWalker.rs operate on the pattern (always valid UTF-8 from JS) with an explicit bounds check, so they're safe as stated.
• Checked that wtf8_byte_sequence_length / decode_wtf8_rune_t still have callers elsewhere — no dead code introduced.

Extended reasoning...

Overview

The PR fixes glob matching over Linux directory entries whose names are not valid UTF-8. The old code advanced ?, the * restart index, and [...] by wtf8_byte_sequence_length(lead_byte) — the nominal length from the lead byte alone — which for ill-formed sequences could step past the end of the slice or swallow the next ASCII byte. The fix adds strings::utf8_codepoint_with_fffd, a thin public wrapper over the existing WebKit-matching convert_utf8_bytes_into_utf16 decoder (ASCII fast path + len.max(1)), and routes the matcher's four path-side call sites through it via decode_rune_at / rune_len_at. A table-driven #[test] covers valid and every ill-formed shape; a Linux-only describe in scan.test.ts creates six files with raw ill-formed byte names and asserts 11 patterns against both hard-coded results and the scan("*").filter(match) invariant.

Security risks

None identified. The change strictly tightens bounds handling — the old code could compute a path_index past path.len(), though downstream < path.len() guards appear to have prevented an actual out-of-bounds read. I verified by tracing convert_utf8_bytes_into_utf16_with_length that the zero-padding of short inputs guarantees the returned len never exceeds bytes.len() (a padded 0x00 always fails the >= 0x80 continuation check before the length would overshoot), and the .max(1) clamp is defensive since no branch of the underlying decoder returns len: 0. No untrusted-length arithmetic, no allocation, no FFI surface added.

Level of scrutiny

Moderate. This is a focused ~60-line bug fix that reuses an existing, in-tree decoder rather than introducing new tables — exactly the "one implementation, in the right place" shape REVIEW.md asks for. But it sits squarely in the "validate representation at every boundary / WTF-8 for real paths" category REVIEW.md flags, and it makes a deliberate semantic shift: the old decode_wtf8_rune_t treated WTF-8-encoded lone surrogates (ED A0..BF ..) as one 3-byte codepoint, while the new path treats them as three separate U+FFFD units to match how to_utf16_alloc renders the returned names. The PR description says this was changed after self-review specifically to make the scan/match invariant hold, and the test pins the ED A0 80 row. That's a defensible choice, but it's a behavior change on a subtle encoding edge that a maintainer should confirm.

Other factors

The remaining wtf8_byte_sequence_length calls in matcher.rs (literal arm) and GlobWalker.rs operate on the pattern, which is always valid UTF-8 from a JS string, and the literal arm bounds-checks pi + n <= path.len() before slicing — so leaving those unchanged is correct, not an oversight. No CODEOWNERS cover the touched files. The bug hunt ran to dry_streak with no findings. The test uses tempDir from harness with manual afterAll cleanup (needed because the directory spans a test.each block), gates on isLinux, and asserts exact sorted arrays plus a self-consistency invariant — strong assertions per REVIEW.md. The known asymmetry (literal � in a pattern compares bytes and won't match ill-formed sequences, while [�] compares codepoints and will) is pre-existing and acknowledged in the PR notes as unfixable at this layer.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I also checked that utf8_codepoint_with_fffd can never return a len past the end of the slice (the zero-padding in convert_utf8_bytes_into_utf16 makes every truncated prefix fail on the first padded byte, so len stops at the real byte count), that all three stepping sites in src/glob/matcher.rs are reached only under a path_index < path.len() guard and the * backtrack recomputes its step from a fresh rune_len_at, and that the other remaining wtf8_byte_sequence_length caller in src/glob/GlobWalker.rs:2162 steps only the pattern (JS-sourced, always valid UTF-8), so it is not affected.

Extended reasoning...

The change swaps the glob matcher's lead-byte-only stepping for a strict maximal-subpart decoder shared from bun_core, adds a Rust unit table for it, and adds a Linux-only scan test over raw invalid-UTF-8 names; it touches no auth, crypto or injection surface. The one open inline finding (the literal arm still compares U+FFFD bytes) plus the matcher being a hot path with byte-index arithmetic over OS-provided names is why a human look remains worthwhile.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟣 src/glob/matcher.rs — Pre-existing, partial fix: a pattern with a literal U+FFFD still misses ill-formed names on Linux, while the class form of the same pattern now matches them. The literal arm at src/glob/matcher.rs:383 compares the pattern's EF BF BD bytes to the raw path bytes, so scan("t�*") returns nothing for t\xc3x even though scan("*") lists it as t�x and Glob.match accepts it. Fix: decode the path side with decode_rune_at in the literal arm too, so every multi-byte literal compares code points the way the [...] arm does; the walker's literal-only fast path can stay byte-based since it never reaches the matcher. The PR description calls this a known gap; it holds for the walker but not for this arm.

  Why this was flagged

  A Linux directory contains an entry whose bytes after t are C3 78. A user calls new Glob("t�*").scanSync({ cwd }), the same name readdir and scan("*") report as t�x. The matcher reaches the literal arm at src/glob/matcher.rs:383 and compares glob[gi..gi+3] (EF BF BD) against path[pi..pi+3] (C3 78 ...) at src/glob/matcher.rs:389-391, which never matches, so the entry is dropped. The sibling class arm at src/glob/matcher.rs:291 now decodes the same path bytes to U+FFFD via decode_rune_at and matches t[�]*, so the two spellings of one pattern disagree on the same name. The base branch also fails the literal spelling, so this is pre-existing; the PR's stated invariant scan(p) == scan("*").filter(match) holds for wildcards and classes only. The author's note says it cannot be fixed in the matcher alone, which is true for the walker's literal-component fast path but not for this arm.

  Verification: Pre-existing, acknowledged in the PR description and the new test comment at test/js/bun/glob/scan.test.ts. The literal arm at src/glob/matcher.rs:383-396 is untouched by the diff and compares EF BF BD against C3 78 .. at line 393, so scan("t�*") misses the entry, while scan("*") lists it as t�x and the class form t[�]* matches via decode_rune_at at matcher.rs:291. Not security-relevant.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[robobun]

Status

Reproduced on bun 1.4.3-canary.1 (367d939) on Linux, with file names that are not valid UTF-8:

• new Bun.Glob("t*.js").scanSync() returns [] for a file named t\xe4\xb8.js.
• bun pm pack --dry-run lists caf\xe9.pem and secret/key\xc3 as packed, although .npmignore has *.pem and secret/*.

The scripts are in the Notes of the description. This branch returns the file and packs neither name. Main (4b02e10) is merged in.

[robobun]

On the literal U+FFFD note in the review above: I left that arm as it is on purpose.

A path component without a wildcard never reaches the matcher, because the walker opens it by its bytes. If only the literal arm decoded the path side, t�* would match a name that t�x still misses. Today both spellings behave the same: a literal U+FFFD matches only a real U+FFFD in the name. The description lists this under Downsides.