Skip to content

Commit c657124

Browse files
committed
Give no twins under an unknown state or an unproved home directory
Under BashInitialStateMode.Unknown, the parser can report an exact value for a word that Bash expands: `-o"$n"` gives `-o$n`, but Bash passes `-oa`. The projection now gives no twins in that mode. Parse is not changed here. A cd without an operand goes to $HOME. Without a live launch HOME, the parser takes that directory from HomeDirectory, but Bash uses its real HOME. The projection now parses the source once more with another HomeDirectory. A command whose exact directory changes gets no twins. The check does not read the cd forms, so it also covers `cd --`, `builtin cd`, `command cd`, and `if cd; then`. Add the inputs as tests and to the real-Bash comparison, which now also compares the working directory. Fix the stale parser comment.
1 parent eab57d5 commit c657124

8 files changed

Lines changed: 246 additions & 16 deletions

File tree

‎IMPLEMENTATION_PLAN.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ priorities.
1616
- [x] Give no twins for an incomplete occurrence, a verb word that is not static, a word without a complete finite value set, an unquoted value that can split or glob, a redirect that depends on a value, and combinations over the limit. Kill a hand mutant of each gate with a test.
1717
- [x] Review round 1: guard a backslash-newline, a carriage return, `=~` and `:~`, an unproved home directory, and `extglob` patterns. Count every twin parse against one budget of parses and source characters. Compare twins with GNU Bash argv on Linux.
1818
- [x] Review round 2: check `=~` and `:~` in the full source. Parse each twin as one command in its exact directory, and check the budget before a twin is built.
19+
- [x] Verification round: give no twins under the Unknown initial state, and for a directory that comes from an unproved `HOME`.
1920
- [ ] Merge the PR after Linux and Windows CI and after the 0.4.0-beta.22 lexer fix. Tag `0.4.0-beta.23` and verify NuGet publication.
2021

2122
## Completed (0.4.0-beta.21 restore the beta.19 verb-slot rule)

‎RELEASE_NOTES.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ This prerelease adds literal twins of Bash commands whose argument words have a
1515
- An unquoted word uses its authored values only under `FreshNonInteractiveNoStartup`, and only when each value is nonempty and has no space, tab, newline, `*`, `?`, `[`, parenthesis, or backslash. The parenthesis rule covers a Bash build with `extglob` on by default.
1616
- A source with a backslash-newline, a carriage return, `=~`, or `:~` anywhere gets no twins. The lexer does not model this text as Bash does. The `=~` and `:~` rule covers a loop list item such as `for f in if=~/x`, and it also stops `[[ $x =~ re ]]`.
1717
- When the source has `~` or `HOME`, a command gets twins only with a live launch `HOME`. Without it, the home directory is the caller assumption `HomeDirectory`.
18-
- Budget: one call parses at most 128 twins and at most 2,048 + 8 x (source length) characters of twin text. The projection checks the budget before it builds a twin, so its cost stays a small constant factor of one `Parse`. On an 80 KB source with 8,000 commands, the projection takes about as long as `Parse`.
18+
- A command whose exact directory comes from `HOME` (after `cd`, `cd --`, `builtin cd`, or `if cd; then`) gets twins only with a live launch `HOME`. The projection finds such a command with one more parse that uses another `HomeDirectory`.
19+
- The projection gives no twins under `BashInitialStateMode.Unknown`. In that mode the parser can report `-o"$n"` as the exact value `-o$n`.
20+
- Budget: one call parses at most 128 twins and at most 2,048 + 8 x (source length) characters of twin text, plus at most one more parse of the source for the `HOME` check. The projection checks the budget before it builds a twin, so its cost stays a small constant factor of one `Parse`. On an 80 KB source with 8,000 commands, the projection takes about as long as `Parse`.
1921
- Values of different words combine independently. The twin set can hold combinations that never run, so a check of every twin is stricter, not weaker.
2022
- A Linux test runs each source and each twin in GNU Bash and compares the argv.
2123
- A twin grants no authority. Existing facts do not change: `Parse` gives the same output as 0.4.0-beta.22.

‎SPEC.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1547,11 +1547,14 @@ command, so read the structure from `SourceOccurrence`.
15471547
The projection uses this schematic flow:
15481548

15491549
```text
1550+
give nothing when the initial state mode is Unknown
15501551
give nothing when the source has a backslash-newline, a carriage return,
15511552
`=~`, or `:~`
15521553
for each occurrence of the full parse:
15531554
give nothing unless the occurrence is complete
15541555
give nothing unless its directory is exact
1556+
without a live launch HOME, give nothing when its directory changes
1557+
in one more parse with another HomeDirectory
15551558
give nothing when it has an assignment prefix (`X=1 cmd`)
15561559
give nothing when the source has `~` or `HOME` and no live launch HOME
15571560
give nothing unless each word has a span in the submitted source
@@ -1609,6 +1612,15 @@ a loop list, so these guards read the full source:
16091612
- Without a live launch `HOME`, the home directory is the caller assumption
16101613
`HomeDirectory`. When the source has `~` or `HOME`, an occurrence gets twins
16111614
only with a live launch `HOME`.
1615+
- A `cd` without an operand (also `cd --`, `builtin cd`, `if cd; then`) goes
1616+
to `$HOME`. Without a live launch `HOME`, the parser takes that directory
1617+
from `HomeDirectory`. The projection does not read the `cd` forms. It parses
1618+
the source once more with another `HomeDirectory`, and an occurrence whose
1619+
exact directory changes gets no twins. When the launch facts export `HOME`
1620+
but `HOME` is not live at the occurrence, the occurrence gets no twins.
1621+
- Under `BashInitialStateMode.Unknown`, the parser can report an exact value
1622+
for a word that Bash expands (`-o"$n"` gives `-o$n`). The projection gives
1623+
no twins in that mode.
16121624

16131625
The budget is checked before a twin text is built. A twin text is one simple
16141626
command with no loop, so its parse costs about as much as its length. The

‎docs/CONSUMER_GUIDE.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1025,7 +1025,9 @@ Rules for a consumer:
10251025
- An unquoted word gets twins only under `FreshNonInteractiveNoStartup`
10261026
with `PublishAuthoredSourceFacts`, and only when no value can split or glob.
10271027
- A source with `~` or `HOME` gets twins only with a live launch `HOME` in
1028-
`LaunchEnvironment`.
1028+
`LaunchEnvironment`. A command whose directory comes from `HOME` (after a
1029+
`cd` without an operand) also needs a live launch `HOME`.
1030+
- The projection gives no twins under `BashInitialStateMode.Unknown`.
10291031

10301032
Warnings:
10311033

‎src/ShellSyntaxTree/BashLiteralTwinProjection.cs‎

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,11 @@ internal static bool TryProject(
209209
projection = null;
210210
cost = default;
211211
var source = parsed.Source;
212-
if (HasUnmodeledText(source))
212+
// SECURITY: with an unknown initial state, the parser can report an
213+
// exact value for a word that Bash expands (`-o"$n"` gives `-o$n`).
214+
// A twin would turn that value into a literal word.
215+
if (options.InitialStateMode == BashInitialStateMode.Unknown ||
216+
HasUnmodeledText(source))
213217
{
214218
return false;
215219
}
@@ -218,10 +222,15 @@ internal static bool TryProject(
218222
var mentionsHome = source.IndexOf('~') >= 0 ||
219223
source.IndexOf("HOME", StringComparison.Ordinal) >= 0;
220224
var candidates = new List<Candidate>();
225+
ParsedCommand? otherHomeParse = null;
221226
for (var index = 0; index < parsed.Commands.Count; index++)
222227
{
223-
if (TryCreateCandidate(source, index, parsed.Commands[index], options,
224-
mentionsHome, out var candidate))
228+
var occurrence = parsed.Commands[index];
229+
if (TryCreateCandidate(source, index, occurrence, options,
230+
mentionsHome, out var candidate) &&
231+
(HasLiveHome(occurrence) ||
232+
HasHomeIndependentDirectory(parsed, options, index, candidate.Directory,
233+
ref otherHomeParse)))
225234
{
226235
candidates.Add(candidate);
227236
}
@@ -272,6 +281,61 @@ internal static bool TryProject(
272281
return true;
273282
}
274283

284+
/// <summary>
285+
/// Returns true when the exact directory of the occurrence does not come
286+
/// from the home directory that the caller assumes.
287+
/// </summary>
288+
/// <remarks>
289+
/// <para>
290+
/// SECURITY: a <c>cd</c> without an operand (also <c>cd --</c>,
291+
/// <c>builtin cd</c>, and <c>if cd; then</c>) changes to <c>$HOME</c>.
292+
/// Without a live launch <c>HOME</c>, the parser takes that directory from
293+
/// <see cref="ShellParserOptions.HomeDirectory"/>, but Bash uses its real
294+
/// <c>HOME</c>. A twin would then claim a wrong directory.
295+
/// </para>
296+
/// <para>
297+
/// The check does not read the <c>cd</c> forms. It parses the source once
298+
/// more with another home directory. When the exact directory of the
299+
/// occurrence changes, the directory depends on the home directory, and
300+
/// the occurrence gets no twins. The check costs at most one more parse
301+
/// for each call. When the launch facts export <c>HOME</c>, the parser
302+
/// needs that value as the home directory, so an occurrence without a live
303+
/// <c>HOME</c> gets no twins.
304+
/// </para>
305+
/// </remarks>
306+
private static bool HasHomeIndependentDirectory(
307+
ParsedCommand parsed,
308+
BashParserOptions options,
309+
int index,
310+
string directory,
311+
ref ParsedCommand? otherHomeParse)
312+
{
313+
if (options.LaunchEnvironment?.ExportedVariables.ContainsKey("HOME") == true)
314+
{
315+
return false;
316+
}
317+
318+
if (otherHomeParse is null)
319+
{
320+
var otherHome = string.Equals(options.HomeDirectory, FirstOtherHome,
321+
StringComparison.Ordinal)
322+
? SecondOtherHome
323+
: FirstOtherHome;
324+
otherHomeParse = new BashParser(options with { HomeDirectory = otherHome })
325+
.Parse(parsed.Source);
326+
}
327+
328+
return !otherHomeParse.IsUnparseable &&
329+
otherHomeParse.Commands.Count == parsed.Commands.Count &&
330+
otherHomeParse.Commands[index].WorkingDirectory is ShellValueDomain.Exact
331+
{ Value: var otherDirectory } &&
332+
string.Equals(otherDirectory, directory, StringComparison.Ordinal);
333+
}
334+
335+
private const string FirstOtherHome = "/nonexistent/shellsyntaxtree-twin-home-1";
336+
337+
private const string SecondOtherHome = "/nonexistent/shellsyntaxtree-twin-home-2";
338+
275339
/// <summary>
276340
/// Returns true when the source has text that the Bash lexer does not
277341
/// model as Bash does.

‎src/ShellSyntaxTree/BashParser.cs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,12 @@ public bool TryProjectFiniteScopes(
6161
/// <summary>
6262
/// Builds literal twins for each command whose changeable argument words
6363
/// (an expansion, a tilde) have a proved finite set of values. A twin
64-
/// writes each such word as one literal value and parses the full source
65-
/// again, so the twin occurrence has the facts of the literal command.
66-
/// Returns false when no command has twins. A command without twins keeps
67-
/// the facts of <see cref="Parse(string)"/> only.
64+
/// writes each such word as one literal value. The parser then parses the
65+
/// text of that one command in the exact directory of the command, so the
66+
/// twin occurrence has the facts of the literal command. Returns false,
67+
/// with a null projection, when no command has twins, and always under
68+
/// <see cref="BashInitialStateMode.Unknown"/>. A command without twins
69+
/// keeps the facts of <see cref="Parse(string)"/> only.
6870
/// </summary>
6971
public bool TryProjectLiteralTwins(
7072
string command,

‎tests/ShellSyntaxTree.Tests/Parsing/BashLiteralTwinBashDifferentialTests.cs‎

Lines changed: 96 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public class BashLiteralTwinBashDifferentialTests
2222
{
2323
private const string Home = "/home/agent";
2424

25-
private static readonly string[] Programs = { "cat", "cmd", "cp", "echo", "gh", "git", "rm" };
25+
private static readonly string[] Programs = { "cat", "cmd", "cp", "echo", "gh", "git", "rm", "tool" };
2626

2727
private static readonly BashParser Parser = new(new BashParserOptions
2828
{
@@ -52,6 +52,11 @@ public class BashLiteralTwinBashDifferentialTests
5252
[InlineData("for n in a b; do cat ~/x \"$n\"; done")]
5353
[InlineData("for n in a b; do cat \"$HOME/x\" \"$n\"; done")]
5454
[InlineData("for r in 100001 100002; do echo -n \"$r: \"; gh run view $r --json headSha 2>/dev/null; echo; done")]
55+
[InlineData("for n in a x; do tool '$n' -o\"$n\"; done")]
56+
[InlineData("for n in a x; do git -C\"$n\" status; done")]
57+
[InlineData("for n in a x; do tool -xo\"$n\"; done")]
58+
[InlineData("for n in a x; do tool -o\"${n}\"; done")]
59+
[InlineData("for n in a x; do tool -o\"$n\"x; done")]
5560
public void Each_twin_has_the_argv_that_bash_passes(string source)
5661
{
5762
Assert.True(Parser.TryProjectLiteralTwins(source, out var projection));
@@ -107,20 +112,103 @@ public void Bash_changes_a_list_item_that_gives_no_twins(
107112
Assert.Equal(bashValue, run.Split('\u001f')[^1]);
108113
}
109114

115+
// With an unknown initial state, the parser reports `-o$n` as an exact
116+
// value, but Bash passes `-oa`. The projection gives no twins in that mode.
117+
[BashTheory]
118+
[InlineData("for n in a; do tool -o\"$n\"; done", "-o$n", "-oa")]
119+
[InlineData("for n in a; do tool -o\"${n}\"; done", "-o${n}", "-oa")]
120+
public void Bash_changes_a_word_that_the_unknown_state_reports_as_exact(
121+
string source,
122+
string parserValue,
123+
string bashValue)
124+
{
125+
var unknown = new BashParser(new BashParserOptions
126+
{
127+
WorkingDirectory = "/work",
128+
HomeDirectory = Home,
129+
PublishAuthoredSourceFacts = true,
130+
});
131+
132+
Assert.False(unknown.TryProjectLiteralTwins(source, out _));
133+
var argument = Assert.Single(Assert.Single(unknown.Parse(source).Commands).Arguments);
134+
Assert.Equal(parserValue, Assert.IsType<ShellValueDomain.Exact>(argument.Value).Value);
135+
Assert.Equal("tool\u001f" + bashValue, Assert.Single(RunArgv(source, "tool")));
136+
}
137+
138+
// A `cd` without an operand goes to $HOME. With a live launch HOME, the
139+
// twin directory is the directory where Bash runs the command. Without
140+
// it, the parser uses the HomeDirectory assumption, so the projection
141+
// gives no twins.
142+
[BashTheory]
143+
[InlineData("cd && for n in a; do cmd \"$n\"; done")]
144+
[InlineData("cd -L && for n in a; do cmd \"$n\"; done")]
145+
[InlineData("cd -- && for n in a; do cmd \"$n\"; done")]
146+
[InlineData("builtin cd && for n in a; do cmd \"$n\"; done")]
147+
[InlineData("command cd && for n in a; do cmd \"$n\"; done")]
148+
[InlineData("cd >/dev/null && for n in a; do cmd \"$n\"; done")]
149+
[InlineData("if cd; then for n in a; do cmd \"$n\"; done; fi")]
150+
[InlineData("for d in x; do cd && cmd \"$d\"; done")]
151+
public void A_cd_to_home_gives_the_directory_that_bash_uses(string source)
152+
{
153+
var home = Directory.CreateTempSubdirectory("sst-twin-home-").FullName;
154+
try
155+
{
156+
var proved = new BashParser(new BashParserOptions
157+
{
158+
WorkingDirectory = "/work",
159+
HomeDirectory = home,
160+
InitialStateMode = BashInitialStateMode.FreshNonInteractiveNoStartup,
161+
PublishAuthoredSourceFacts = true,
162+
LaunchEnvironment = new ShellLaunchEnvironment(
163+
new[] { new KeyValuePair<string, string>("HOME", home) },
164+
Array.Empty<string>()),
165+
});
166+
Assert.True(proved.TryProjectLiteralTwins(source, out var projection));
167+
var twin = Assert.Single(Assert.Single(projection!.Commands).Twins);
168+
var run = Assert.Single(RunRecords(source, "cmd", home));
169+
Assert.Equal(SstArgv(twin), run.Argv);
170+
Assert.Equal(twin.WorkingDirectory, run.Directory);
171+
172+
var assumed = new BashParser(new BashParserOptions
173+
{
174+
WorkingDirectory = "/work",
175+
HomeDirectory = "/home/assumed",
176+
InitialStateMode = BashInitialStateMode.FreshNonInteractiveNoStartup,
177+
PublishAuthoredSourceFacts = true,
178+
});
179+
Assert.False(assumed.TryProjectLiteralTwins(source, out _));
180+
var occurrence = assumed.Parse(source).Commands.Single(command =>
181+
command.Clause.Elements[0].Value == "cmd");
182+
Assert.Equal("/home/assumed",
183+
Assert.IsType<ShellValueDomain.Exact>(occurrence.WorkingDirectory).Value);
184+
Assert.NotEqual("/home/assumed", run.Directory);
185+
}
186+
finally
187+
{
188+
Directory.Delete(home, recursive: true);
189+
}
190+
}
191+
110192
private static string SstArgv(BashLiteralTwin twin) =>
111193
string.Join("\u001f", twin.Occurrence.Clause.Elements
112194
.Where(element => element.Role != ClauseElementRole.Redirect)
113195
.Select(element => element.Value));
114196

115-
private static IReadOnlyList<string> RunArgv(string source, string program)
197+
private static IReadOnlyList<string> RunArgv(string source, string program) =>
198+
RunRecords(source, program, Home).Select(record => record.Argv).ToArray();
199+
200+
private static IReadOnlyList<(string Argv, string Directory)> RunRecords(
201+
string source,
202+
string program,
203+
string home)
116204
{
117205
var script = new StringBuilder("exec 3>&1\n");
118206
foreach (var name in Programs)
119207
{
120208
script.Append(name)
121209
.Append("() { builtin printf '%s\\037' ")
122210
.Append(name)
123-
.Append(" \"$@\" >&3; builtin printf '\\036' >&3; }\n");
211+
.Append(" \"$@\" >&3; builtin printf '\\035%s\\036' \"$PWD\" >&3; }\n");
124212
}
125213

126214
script.Append(source).Append('\n');
@@ -139,7 +227,7 @@ private static IReadOnlyList<string> RunArgv(string source, string program)
139227
start.ArgumentList.Add("-c");
140228
start.ArgumentList.Add(script.ToString());
141229
start.Environment.Clear();
142-
start.Environment["HOME"] = Home;
230+
start.Environment["HOME"] = home;
143231
start.Environment["PATH"] = "/nonexistent-shellsyntaxtree";
144232

145233
using var process = Process.Start(start)!;
@@ -149,8 +237,10 @@ private static IReadOnlyList<string> RunArgv(string source, string program)
149237
Assert.Equal(string.Empty, error.Result);
150238

151239
return output.Split('\u001e', StringSplitOptions.RemoveEmptyEntries)
152-
.Select(record => record.EndsWith('\u001f') ? record[..^1] : record)
153-
.Where(record => record.Split('\u001f')[0] == program)
240+
.Select(record => record.Split('\u001d'))
241+
.Select(parts => (Argv: parts[0].EndsWith('\u001f') ? parts[0][..^1] : parts[0],
242+
Directory: parts[1]))
243+
.Where(record => record.Argv.Split('\u001f')[0] == program)
154244
.ToArray();
155245
}
156246
finally

0 commit comments

Comments
 (0)