Repository navigation
Floor the temp-name truncation to a char boundary - #18
Open
kridaydave wants to merge 1 commit into
Open
kridaydave wants to merge 1 commit into
kridaydave wants to merge 1 commit into
Conversation
A file name longer than 128 bytes whose 128th byte falls inside a multi-byte character panicked `oot update` with exit 101, partway through materializing the tree.
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.
oot updatepanics with exit 101 when a stored file's name is longer than 128 bytes and byte 128 falls inside a multi-byte character. The truncation that guards against ENAMETOOLONG sliced aStringat a fixed byte index.Red-first proof, run against pristine
origin/mainsource:With the change, that same test binary reports
1 passed; 0 failed.The trigger is a 129-byte name of 127 ASCII characters plus one 2-byte character. The panic lands inside
write_file_atomic, so it fires partway through the write loop and leaves the working copy half-written: onorigin/mainthe long-named file came back MISSING afterwards, and the temp name it had already deleted files to make room for was never written.Diff
One line of logic.
full_name.floor_char_boundary(128)keeps the same 128-byte budget and lands on a valid boundary instead of panicking.Not shipped
.peminvisibility.toml:14does not match a real certificate.path_is_privatedoes segment-boundary matching, so.pemonly matches a file literally named.pem, notcerts/server.pem. Fail-open on the export path. Fix is either the config (*.pem) or a suffix rule in the matcher. Not shipped here because the right answer is a policy decision, and it belongs with the duplicated private-branch matcher below.visibility.rs:107branch_is_privateuses segment boundaries;visibility.rs:203-212incheck()uses a bareString::containsand does not trim the pattern. Measured:private_branches = ["internal-audit"]givesexport_drops=false, adjudicate_flags=trueforinternal-audit-refactorandhotfix-internal-audit. A trailing space in the config makes adjudication cloak a branch that export publishes.visibility.rs:247-252divides epoch seconds by 86400 with no local offset, so the embargo lifts up to 12 hours early in western timezones.