Repository navigation
fix: read the edge shapes of the unguarded variable path guard entry - #909
Merged
Merged
Conversation
The rm-unguarded-variable-path entry misjudged three shapes its review found.
Inside a double-quoted shell string the successor's own rewrite was refused.
The string's re-read writes ${X:?} out as the ${X} it prints, which alone can
be empty, so sh -c "rm -rf ${X:?}/y" read as the shape its guard rules out.
The tokenizer now keeps each word holding a guarded value, and the payload
re-read takes its lead from the string written with each such value as its
guard, asked only where the plain reading leads. A site that can also print
the empty text keeps its old spelling, so ${X:-}/y still refuses, and the
arg_values compare reads the same spelling as before.
A guard nested in a default was not read, so "${VAR:-${OTHER:?}}"/y and
"${VAR:-${OTHER:-/tmp}}"/y refused. The guarded values of an expansion are now
every name a :?, :- or := form guards anywhere in it, less any name it also
writes another way. That also closes "${VAR:-${VAR}}"/y, which the old exact
text skip allowed although it prints VAR's empty value.
A variable followed by a command substitution before the slash was not
followed, so rm -rf $VAR$(true)/x was allowed. The site is now followed past
substitution output, backticks included. Arithmetic expansion always prints a
number and stays allowed.
Resolves: iss-2610100938485695
Assisted-by: Claude:claude-opus-5-5
…edges # Conflicts: # .abcd/work/issues/open/iss-2610100938485695-the-rm-unguarded-variable-path-guard-entry-misjudges-three.md # .abcd/work/issues/open/iss-2610100938485695-the-rm-unguarded-variable-path-guard.md # .abcd/work/issues/resolved/iss-2610100938485695-the-rm-unguarded-variable-path-guard-entry-misjudges-three.md
An unquoted default holding a blank leaves the `/` opening a field of its
own: `rm -rf ${X:- }/y` with X unset is `rm -rf /y` in bash, sh and dash,
and `${X:-a }/y` is `a` and `/y`. The entry now reads only the text after
an unquoted site's last blank, so both refuse; a quoted, escaped or ANSI-C
blank stays text and stays allowed.
Inside a double-quoted shell string the guarded rewrite hid the same
defaults from the re-read: `sh -c "rm -rf ${X:- }/y"` and
`sh -c "rm -rf ${X:-\"\"}/y"` were written as `${X:?}` and allowed,
although the string's shell splits the blank and takes the quotes off. A
site is now written as its guard only when no text it prints can be emptied
there; otherwise it keeps `${X}`, which refuses as main did.
Refs: iss-2610100938485695
Assisted-by: Claude:claude-opus-5-5
Refs: iss-2610110025455728 Assisted-by: Claude:claude-opus-5-5
A default the enclosing shell writes as an escaped substitution is run by
the string's shell, which can print nothing: `sh -c "rm -rf ${X:-\$(true)}/y"`
hands it `rm -rf $(true)/y`. The text read as one that cannot be emptied, so
the site was written out as `${X:?}` and allowed, where main refused it. A
`$` or backquote that opens nothing the guard follows is now read as able to
print nothing.
A default written with a blank inside the string's own quotes
(`sh -c "rm -rf \"${X:-my dir}\"/y"`) is still refused, as main refuses it:
the site cannot see the quotes the string's shell will read.
Refs: iss-2610100938485695
Assisted-by: Claude:claude-opus-5-5
Refs: iss-2610110025455728 Assisted-by: Claude:claude-opus-5-5
emptiedInside reads whether a default's text opens with a backquote, a command substitution the shell string's own shell runs. That is shell grammar, not markdown, as the guard's other allowlisted files are. Assisted-by: Claude:claude-opus-5-5
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
1 participant
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
The
rm-unguarded-variable-pathguard entry from #889 now reads the three edge shapes its review found, plus two more found in this branch's review.sh -c "rm -rf ${X:?}/y"was refused, because the string's re-read spelled${X:?}as${X}. It is now allowed, and so is a nested guard (${VAR:-${OTHER:?}}).rm -rf $VAR$(true)/xis now refused.rm -rf ${X:- }/yintorm -rf /y, and${X:-a }/yintoaand/y. Both are now refused. This was allowed since fix: refuse an rm whose path starts with a variable that can be empty #889. A quoted, escaped or$'\x20'blank stays text and stays allowed.sh -c "rm -rf ${X:-\"\"}/y",${X:- }and${X:-\$(true)}, the inner shell strips the quotes, splits the blank or runs the substitution. The site is written as its guard only when nothing it prints can be emptied there; otherwise it keeps${X}and refuses, as main does.Not in this change
sh -c "rm -rf ${X:+x}/y"andsh -c "rm -rf ${X:-\${Y}}/y"are still allowed, as they are on main. Both come from how the string's re-read pairs a word it writes out, which is a separate mechanism, captured in the record below.sh -c "rm -rf ${X:-\\ }/y"andsh -c "rm -rf \"${X:-my dir}\"/y"are refused although safe, as on main. That is deliberate.Merge with main
This branch merged #904's slug rename. The record it resolves had a rename/rename conflict: main shortened its slug in
open/while this branch moved it toresolved/. It is resolved by hand intoresolved/at the short slug.record-slug-rename -applythen found nothing to rename.Review
A Fable review of the three-shape fix confirmed each shape against the parent. It went silent partway through probing, so I finished that probe on a scratch copy. That probe found the split-default regression.
A second Fable review of the split fix returned FIX-FIRST on the substitution case. That fix is in 8d3cf67, with its tests watched fail first.
Resolves: iss-2610100938485695
Refs: iss-2610110025455728
Refs: iss-2610091942156774
Assisted-by: Claude:claude-opus-5-5